From aa721d1e7db973c736f65f079ba28828f9c25935 Mon Sep 17 00:00:00 2001 From: "Bryan C. Mills" Date: Thu, 11 Jan 2024 15:00:17 -0500 Subject: [PATCH] [release-branch.go1.22] cmd/go/internal/toolchain: apply the -modcacherw flag when downloading a module to determine what toolchain it needs Fixes #64282. Change-Id: I3f211c599ee70cb58254d0bc07eeb3c135124e58 Reviewed-on: https://go-review.googlesource.com/c/go/+/555436 Auto-Submit: Bryan Mills Reviewed-by: Russ Cox (cherry picked from commit cc38c68ae09fa591697a4239a7dedd2efe386995) Reviewed-on: https://go-review.googlesource.com/c/go/+/559218 LUCI-TryBot-Result: Go LUCI --- src/cmd/go/internal/toolchain/select.go | 148 ++++++++++++------ .../script/install_modcacherw_issue64282.txt | 45 ++++++ 2 files changed, 149 insertions(+), 44 deletions(-) create mode 100644 src/cmd/go/testdata/script/install_modcacherw_issue64282.txt diff --git a/src/cmd/go/internal/toolchain/select.go b/src/cmd/go/internal/toolchain/select.go index 9fd1549a61bbc1..dcf3be92ccd84c 100644 --- a/src/cmd/go/internal/toolchain/select.go +++ b/src/cmd/go/internal/toolchain/select.go @@ -8,6 +8,7 @@ package toolchain import ( "context" "errors" + "flag" "fmt" "go/build" "io/fs" @@ -24,6 +25,7 @@ import ( "cmd/go/internal/modfetch" "cmd/go/internal/modload" "cmd/go/internal/run" + "cmd/go/internal/work" "golang.org/x/mod/module" ) @@ -486,74 +488,132 @@ func goInstallVersion() bool { // Note: We assume there are no flags between 'go' and 'install' or 'run'. // During testing there are some debugging flags that are accepted // in that position, but in production go binaries there are not. - if len(os.Args) < 3 || (os.Args[1] != "install" && os.Args[1] != "run") { + if len(os.Args) < 3 { return false } - // Check for pkg@version. - var arg string + var cmdFlags *flag.FlagSet switch os.Args[1] { default: + // Command doesn't support a pkg@version as the main module. return false case "install": - // We would like to let 'go install -newflag pkg@version' work even - // across a toolchain switch. To make that work, assume the pkg@version - // is the last argument and skip the flag parsing. - arg = os.Args[len(os.Args)-1] + cmdFlags = &work.CmdInstall.Flag case "run": - // For run, the pkg@version can be anywhere on the command line, - // because it is preceded by run flags and followed by arguments to the - // program being run. To handle that precisely, we have to interpret the - // flags a little bit, to know whether each flag takes an optional argument. - // We can still allow unknown flags as long as they have an explicit =value. - args := os.Args[2:] - for i := 0; i < len(args); i++ { - a := args[i] - if !strings.HasPrefix(a, "-") { - arg = a - break - } - if a == "-" { - // non-flag but also non-pkg@version + cmdFlags = &run.CmdRun.Flag + } + + // The modcachrw flag is unique, in that it affects how we fetch the + // requested module to even figure out what toolchain it needs. + // We need to actually set it before we check the toolchain version. + // (See https://go.dev/issue/64282.) + modcacherwFlag := cmdFlags.Lookup("modcacherw") + if modcacherwFlag == nil { + base.Fatalf("internal error: modcacherw flag not registered for command") + } + modcacherwVal, ok := modcacherwFlag.Value.(interface { + IsBoolFlag() bool + flag.Value + }) + if !ok || !modcacherwVal.IsBoolFlag() { + base.Fatalf("internal error: modcacherw is not a boolean flag") + } + + // Make a best effort to parse the command's args to find the pkg@version + // argument and the -modcacherw flag. + var ( + pkgArg string + modcacherwSeen bool + ) + for args := os.Args[2:]; len(args) > 0; { + a := args[0] + args = args[1:] + if a == "--" { + if len(args) == 0 { return false } - if a == "--" { - if i+1 >= len(args) { - return false - } - arg = args[i+1] - break + pkgArg = args[0] + break + } + + a, ok := strings.CutPrefix(a, "-") + if !ok { + // Not a flag argument. Must be a package. + pkgArg = a + break + } + a = strings.TrimPrefix(a, "-") // Treat --flag as -flag. + + name, val, hasEq := strings.Cut(a, "=") + + if name == "modcacherw" { + if !hasEq { + val = "true" } - a = strings.TrimPrefix(a, "-") - a = strings.TrimPrefix(a, "-") - if strings.HasPrefix(a, "-") { - // non-flag but also non-pkg@version + if err := modcacherwVal.Set(val); err != nil { return false } - if strings.Contains(a, "=") { - // already has value - continue - } - f := run.CmdRun.Flag.Lookup(a) - if f == nil { - // Unknown flag. Give up. The command is going to fail in flag parsing. + modcacherwSeen = true + continue + } + + if hasEq { + // Already has a value; don't bother parsing it. + continue + } + + f := run.CmdRun.Flag.Lookup(a) + if f == nil { + // We don't know whether this flag is a boolean. + if os.Args[1] == "run" { + // We don't know where to find the pkg@version argument. + // For run, the pkg@version can be anywhere on the command line, + // because it is preceded by run flags and followed by arguments to the + // program being run. Since we don't know whether this flag takes + // an argument, we can't reliably identify the end of the run flags. + // Just give up and let the user clarify using the "=" form.. return false } - if bf, ok := f.Value.(interface{ IsBoolFlag() bool }); ok && bf.IsBoolFlag() { - // Does not take value. - continue + + // We would like to let 'go install -newflag pkg@version' work even + // across a toolchain switch. To make that work, assume by default that + // the pkg@version is the last argument and skip the remaining args unless + // we spot a plausible "-modcacherw" flag. + for len(args) > 0 { + a := args[0] + name, _, _ := strings.Cut(a, "=") + if name == "-modcacherw" || name == "--modcacherw" { + break + } + if len(args) == 1 && !strings.HasPrefix(a, "-") { + pkgArg = a + } + args = args[1:] } - i++ // Does take a value; skip it. + continue + } + + if bf, ok := f.Value.(interface{ IsBoolFlag() bool }); !ok || !bf.IsBoolFlag() { + // The next arg is the value for this flag. Skip it. + args = args[1:] + continue } } - if !strings.Contains(arg, "@") || build.IsLocalImport(arg) || filepath.IsAbs(arg) { + + if !strings.Contains(pkgArg, "@") || build.IsLocalImport(pkgArg) || filepath.IsAbs(pkgArg) { return false } - path, version, _ := strings.Cut(arg, "@") + path, version, _ := strings.Cut(pkgArg, "@") if path == "" || version == "" || gover.IsToolchain(path) { return false } + if !modcacherwSeen && base.InGOFLAGS("-modcacherw") { + fs := flag.NewFlagSet("goInstallVersion", flag.ExitOnError) + fs.Var(modcacherwVal, "modcacherw", modcacherwFlag.Usage) + base.SetFromGOFLAGS(fs) + } + // It would be correct to simply return true here, bypassing use // of the current go.mod or go.work, and let "go run" or "go install" // do the rest, including a toolchain switch. diff --git a/src/cmd/go/testdata/script/install_modcacherw_issue64282.txt b/src/cmd/go/testdata/script/install_modcacherw_issue64282.txt new file mode 100644 index 00000000000000..3e1e6e562a3f7a --- /dev/null +++ b/src/cmd/go/testdata/script/install_modcacherw_issue64282.txt @@ -0,0 +1,45 @@ +# Regression test for https://go.dev/issue/64282. +# +# 'go install' and 'go run' with pkg@version arguments should make +# a best effort to parse flags relevant to downloading modules +# (currently only -modcacherw) before actually downloading the module +# to identify which toolchain version to use. +# +# However, the best-effort flag parsing should not interfere with +# actual flag parsing if we don't switch toolchains. In particular, +# unrecognized flags should still be diagnosed after the module for +# the requested package has been downloaded and checked for toolchain +# upgrades. + + +! go install -cake=delicious -modcacherw example.com/printversion@v0.1.0 +stderr '^flag provided but not defined: -cake$' + # Because the -modcacherw flag was set, we should be able to modify the contents + # of a directory within the module cache. +cp $WORK/extraneous.txt $GOPATH/pkg/mod/example.com/printversion@v0.1.0/extraneous_file.go +go clean -modcache + + +! go install -unknownflag -tags -modcacherw example.com/printversion@v0.1.0 +stderr '^flag provided but not defined: -unknownflag$' +cp $WORK/extraneous.txt $GOPATH/pkg/mod/example.com/printversion@v0.1.0/extraneous_file.go +go clean -modcache + + +# Also try it with a 'go install' that succeeds. +# (But skip in short mode, because linking a binary is expensive.) +[!short] go install -modcacherw example.com/printversion@v0.1.0 +[!short] cp $WORK/extraneous.txt $GOPATH/pkg/mod/example.com/printversion@v0.1.0/extraneous_file.go +[!short] go clean -modcache + + +# The flag should also be applied if given in GOFLAGS +# instead of on the command line. +env GOFLAGS=-modcacherw +! go install -cake=delicious example.com/printversion@v0.1.0 +stderr '^flag provided but not defined: -cake$' +cp $WORK/extraneous.txt $GOPATH/pkg/mod/example.com/printversion@v0.1.0/extraneous_file.go + + +-- $WORK/extraneous.txt -- +This is not a Go source file.