Skip to content

Commit cc38c68

Browse files
Bryan C. Millsgopherbot
Bryan C. Mills
authored and
gopherbot
committed
cmd/go/internal/toolchain: apply the -modcacherw flag when downloading a module to determine what toolchain it needs
Fixes #64282. Change-Id: I3f211c599ee70cb58254d0bc07eeb3c135124e58 Cq-Include-Trybots: luci.golang.try:gotip-linux-amd64-longtest,gotip-windows-amd64-longtest Reviewed-on: https://go-review.googlesource.com/c/go/+/555436 Auto-Submit: Bryan Mills <[email protected]> LUCI-TryBot-Result: Go LUCI <[email protected]> Reviewed-by: Russ Cox <[email protected]>
1 parent 653abef commit cc38c68

2 files changed

Lines changed: 149 additions & 44 deletions

File tree

‎src/cmd/go/internal/toolchain/select.go‎

Lines changed: 104 additions & 44 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ package toolchain
88
import (
99
"context"
1010
"errors"
11+
"flag"
1112
"fmt"
1213
"go/build"
1314
"io/fs"
@@ -24,6 +25,7 @@ import (
2425
"cmd/go/internal/modfetch"
2526
"cmd/go/internal/modload"
2627
"cmd/go/internal/run"
28+
"cmd/go/internal/work"
2729

2830
"golang.org/x/mod/module"
2931
)
@@ -486,74 +488,132 @@ func goInstallVersion() bool {
486488
// Note: We assume there are no flags between 'go' and 'install' or 'run'.
487489
// During testing there are some debugging flags that are accepted
488490
// in that position, but in production go binaries there are not.
489-
if len(os.Args) < 3 || (os.Args[1] != "install" && os.Args[1] != "run") {
491+
if len(os.Args) < 3 {
490492
return false
491493
}
492494

493-
// Check for pkg@version.
494-
var arg string
495+
var cmdFlags *flag.FlagSet
495496
switch os.Args[1] {
496497
default:
498+
// Command doesn't support a pkg@version as the main module.
497499
return false
498500
case "install":
499-
// We would like to let 'go install -newflag pkg@version' work even
500-
// across a toolchain switch. To make that work, assume the pkg@version
501-
// is the last argument and skip the flag parsing.
502-
arg = os.Args[len(os.Args)-1]
501+
cmdFlags = &work.CmdInstall.Flag
503502
case "run":
504-
// For run, the pkg@version can be anywhere on the command line,
505-
// because it is preceded by run flags and followed by arguments to the
506-
// program being run. To handle that precisely, we have to interpret the
507-
// flags a little bit, to know whether each flag takes an optional argument.
508-
// We can still allow unknown flags as long as they have an explicit =value.
509-
args := os.Args[2:]
510-
for i := 0; i < len(args); i++ {
511-
a := args[i]
512-
if !strings.HasPrefix(a, "-") {
513-
arg = a
514-
break
515-
}
516-
if a == "-" {
517-
// non-flag but also non-pkg@version
503+
cmdFlags = &run.CmdRun.Flag
504+
}
505+
506+
// The modcachrw flag is unique, in that it affects how we fetch the
507+
// requested module to even figure out what toolchain it needs.
508+
// We need to actually set it before we check the toolchain version.
509+
// (See https://go.dev/issue/64282.)
510+
modcacherwFlag := cmdFlags.Lookup("modcacherw")
511+
if modcacherwFlag == nil {
512+
base.Fatalf("internal error: modcacherw flag not registered for command")
513+
}
514+
modcacherwVal, ok := modcacherwFlag.Value.(interface {
515+
IsBoolFlag() bool
516+
flag.Value
517+
})
518+
if !ok || !modcacherwVal.IsBoolFlag() {
519+
base.Fatalf("internal error: modcacherw is not a boolean flag")
520+
}
521+
522+
// Make a best effort to parse the command's args to find the pkg@version
523+
// argument and the -modcacherw flag.
524+
var (
525+
pkgArg string
526+
modcacherwSeen bool
527+
)
528+
for args := os.Args[2:]; len(args) > 0; {
529+
a := args[0]
530+
args = args[1:]
531+
if a == "--" {
532+
if len(args) == 0 {
518533
return false
519534
}
520-
if a == "--" {
521-
if i+1 >= len(args) {
522-
return false
523-
}
524-
arg = args[i+1]
525-
break
535+
pkgArg = args[0]
536+
break
537+
}
538+
539+
a, ok := strings.CutPrefix(a, "-")
540+
if !ok {
541+
// Not a flag argument. Must be a package.
542+
pkgArg = a
543+
break
544+
}
545+
a = strings.TrimPrefix(a, "-") // Treat --flag as -flag.
546+
547+
name, val, hasEq := strings.Cut(a, "=")
548+
549+
if name == "modcacherw" {
550+
if !hasEq {
551+
val = "true"
526552
}
527-
a = strings.TrimPrefix(a, "-")
528-
a = strings.TrimPrefix(a, "-")
529-
if strings.HasPrefix(a, "-") {
530-
// non-flag but also non-pkg@version
553+
if err := modcacherwVal.Set(val); err != nil {
531554
return false
532555
}
533-
if strings.Contains(a, "=") {
534-
// already has value
535-
continue
536-
}
537-
f := run.CmdRun.Flag.Lookup(a)
538-
if f == nil {
539-
// Unknown flag. Give up. The command is going to fail in flag parsing.
556+
modcacherwSeen = true
557+
continue
558+
}
559+
560+
if hasEq {
561+
// Already has a value; don't bother parsing it.
562+
continue
563+
}
564+
565+
f := run.CmdRun.Flag.Lookup(a)
566+
if f == nil {
567+
// We don't know whether this flag is a boolean.
568+
if os.Args[1] == "run" {
569+
// We don't know where to find the pkg@version argument.
570+
// For run, the pkg@version can be anywhere on the command line,
571+
// because it is preceded by run flags and followed by arguments to the
572+
// program being run. Since we don't know whether this flag takes
573+
// an argument, we can't reliably identify the end of the run flags.
574+
// Just give up and let the user clarify using the "=" form..
540575
return false
541576
}
542-
if bf, ok := f.Value.(interface{ IsBoolFlag() bool }); ok && bf.IsBoolFlag() {
543-
// Does not take value.
544-
continue
577+
578+
// We would like to let 'go install -newflag pkg@version' work even
579+
// across a toolchain switch. To make that work, assume by default that
580+
// the pkg@version is the last argument and skip the remaining args unless
581+
// we spot a plausible "-modcacherw" flag.
582+
for len(args) > 0 {
583+
a := args[0]
584+
name, _, _ := strings.Cut(a, "=")
585+
if name == "-modcacherw" || name == "--modcacherw" {
586+
break
587+
}
588+
if len(args) == 1 && !strings.HasPrefix(a, "-") {
589+
pkgArg = a
590+
}
591+
args = args[1:]
545592
}
546-
i++ // Does take a value; skip it.
593+
continue
594+
}
595+
596+
if bf, ok := f.Value.(interface{ IsBoolFlag() bool }); !ok || !bf.IsBoolFlag() {
597+
// The next arg is the value for this flag. Skip it.
598+
args = args[1:]
599+
continue
547600
}
548601
}
549-
if !strings.Contains(arg, "@") || build.IsLocalImport(arg) || filepath.IsAbs(arg) {
602+
603+
if !strings.Contains(pkgArg, "@") || build.IsLocalImport(pkgArg) || filepath.IsAbs(pkgArg) {
550604
return false
551605
}
552-
path, version, _ := strings.Cut(arg, "@")
606+
path, version, _ := strings.Cut(pkgArg, "@")
553607
if path == "" || version == "" || gover.IsToolchain(path) {
554608
return false
555609
}
556610

611+
if !modcacherwSeen && base.InGOFLAGS("-modcacherw") {
612+
fs := flag.NewFlagSet("goInstallVersion", flag.ExitOnError)
613+
fs.Var(modcacherwVal, "modcacherw", modcacherwFlag.Usage)
614+
base.SetFromGOFLAGS(fs)
615+
}
616+
557617
// It would be correct to simply return true here, bypassing use
558618
// of the current go.mod or go.work, and let "go run" or "go install"
559619
// do the rest, including a toolchain switch.

‎src/cmd/go/testdata/script/install_modcacherw_issue64282.txt‎

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,45 @@
1+
# Regression test for https://go.dev/issue/64282.
2+
#
3+
# 'go install' and 'go run' with pkg@version arguments should make
4+
# a best effort to parse flags relevant to downloading modules
5+
# (currently only -modcacherw) before actually downloading the module
6+
# to identify which toolchain version to use.
7+
#
8+
# However, the best-effort flag parsing should not interfere with
9+
# actual flag parsing if we don't switch toolchains. In particular,
10+
# unrecognized flags should still be diagnosed after the module for
11+
# the requested package has been downloaded and checked for toolchain
12+
# upgrades.
13+
14+
15+
! go install -cake=delicious -modcacherw example.com/[email protected]
16+
stderr '^flag provided but not defined: -cake$'
17+
# Because the -modcacherw flag was set, we should be able to modify the contents
18+
# of a directory within the module cache.
19+
cp $WORK/extraneous.txt $GOPATH/pkg/mod/example.com/[email protected]/extraneous_file.go
20+
go clean -modcache
21+
22+
23+
! go install -unknownflag -tags -modcacherw example.com/[email protected]
24+
stderr '^flag provided but not defined: -unknownflag$'
25+
cp $WORK/extraneous.txt $GOPATH/pkg/mod/example.com/[email protected]/extraneous_file.go
26+
go clean -modcache
27+
28+
29+
# Also try it with a 'go install' that succeeds.
30+
# (But skip in short mode, because linking a binary is expensive.)
31+
[!short] go install -modcacherw example.com/[email protected]
32+
[!short] cp $WORK/extraneous.txt $GOPATH/pkg/mod/example.com/[email protected]/extraneous_file.go
33+
[!short] go clean -modcache
34+
35+
36+
# The flag should also be applied if given in GOFLAGS
37+
# instead of on the command line.
38+
env GOFLAGS=-modcacherw
39+
! go install -cake=delicious example.com/[email protected]
40+
stderr '^flag provided but not defined: -cake$'
41+
cp $WORK/extraneous.txt $GOPATH/pkg/mod/example.com/[email protected]/extraneous_file.go
42+
43+
44+
-- $WORK/extraneous.txt --
45+
This is not a Go source file.

0 commit comments

Comments
 (0)