From 884179022e0f57d6cf26a5fb12695a33dc1780fc Mon Sep 17 00:00:00 2001 From: Daniel Martí Date: Thu, 10 Sep 2020 22:53:59 +0100 Subject: [release-branch.go1.15] cmd/go: relax version's error on unexpected flags MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit In https://golang.org/cl/221397 we made commands like "go version -v" error, since both of the command's flags only make sense when arguments follow them. Without arguments, the command only reports Go's own version, and the flags are most likely a mistake. However, the script below is entirely reasonable: export GOFLAGS=-v # make all Go commands verbose go version go build After the previous CL, "go version" would error. Instead, only error if the flag was passed explicitly, and not via GOFLAGS. The patch does mean that we won't error on "GOFLAGS=-v go version -v", but that very unlikely false negative is okay. The error is only meant to help the user not misuse the flags, anyway - it's not a critical error of any sort. To reuse inGOFLAGS, we move it to the base package and export it there, since it's where the rest of the GOFLAGS funcs are. Fixes #41464. Change-Id: I74003dd25d94bacf9ac507b5cad778fd65233321 Reviewed-on: https://go-review.googlesource.com/c/go/+/254157 Trust: Daniel Martí Run-TryBot: Daniel Martí TryBot-Result: Go Bot Reviewed-by: Bryan C. Mills (cherry picked from commit de0957dc081e1ec49c99a0f37403ceadbaaedf85) Reviewed-on: https://go-review.googlesource.com/c/go/+/255498 Trust: Bryan C. Mills Run-TryBot: Bryan C. Mills --- src/cmd/go/internal/base/goflags.go | 17 +++++++++++++++++ src/cmd/go/internal/version/version.go | 9 ++++++++- src/cmd/go/internal/work/init.go | 22 +++------------------- src/cmd/go/testdata/script/version.txt | 6 ++++++ 4 files changed, 34 insertions(+), 20 deletions(-) diff --git a/src/cmd/go/internal/base/goflags.go b/src/cmd/go/internal/base/goflags.go index 34766134b0..f29cc3cf9a 100644 --- a/src/cmd/go/internal/base/goflags.go +++ b/src/cmd/go/internal/base/goflags.go @@ -153,3 +153,20 @@ func SetFromGOFLAGS(flags *flag.FlagSet) { } } } + +// InGOFLAGS returns whether GOFLAGS contains the given flag, such as "-mod". +func InGOFLAGS(flag string) bool { + for _, goflag := range GOFLAGS() { + name := goflag + if strings.HasPrefix(name, "--") { + name = name[1:] + } + if i := strings.Index(name, "="); i >= 0 { + name = name[:i] + } + if name == flag { + return true + } + } + return false +} diff --git a/src/cmd/go/internal/version/version.go b/src/cmd/go/internal/version/version.go index ac2ae50155..a5b13653cd 100644 --- a/src/cmd/go/internal/version/version.go +++ b/src/cmd/go/internal/version/version.go @@ -53,7 +53,14 @@ var ( func runVersion(cmd *base.Command, args []string) { if len(args) == 0 { - if *versionM || *versionV { + // If any of this command's flags were passed explicitly, error + // out, because they only make sense with arguments. + // + // Don't error if the flags came from GOFLAGS, since that can be + // a reasonable use case. For example, imagine GOFLAGS=-v to + // turn "verbose mode" on for all Go commands, which should not + // break "go version". + if (!base.InGOFLAGS("-m") && *versionM) || (!base.InGOFLAGS("-v") && *versionV) { fmt.Fprintf(os.Stderr, "go version: flags can only be used with arguments\n") base.SetExitStatus(2) return diff --git a/src/cmd/go/internal/work/init.go b/src/cmd/go/internal/work/init.go index dad3b10111..c168364cd1 100644 --- a/src/cmd/go/internal/work/init.go +++ b/src/cmd/go/internal/work/init.go @@ -254,34 +254,18 @@ func buildModeInit() { case "": // ok case "readonly", "vendor", "mod": - if !cfg.ModulesEnabled && !inGOFLAGS("-mod") { + if !cfg.ModulesEnabled && !base.InGOFLAGS("-mod") { base.Fatalf("build flag -mod=%s only valid when using modules", cfg.BuildMod) } default: base.Fatalf("-mod=%s not supported (can be '', 'mod', 'readonly', or 'vendor')", cfg.BuildMod) } if !cfg.ModulesEnabled { - if cfg.ModCacheRW && !inGOFLAGS("-modcacherw") { + if cfg.ModCacheRW && !base.InGOFLAGS("-modcacherw") { base.Fatalf("build flag -modcacherw only valid when using modules") } - if cfg.ModFile != "" && !inGOFLAGS("-mod") { + if cfg.ModFile != "" && !base.InGOFLAGS("-mod") { base.Fatalf("build flag -modfile only valid when using modules") } } } - -func inGOFLAGS(flag string) bool { - for _, goflag := range base.GOFLAGS() { - name := goflag - if strings.HasPrefix(name, "--") { - name = name[1:] - } - if i := strings.Index(name, "="); i >= 0 { - name = name[:i] - } - if name == flag { - return true - } - } - return false -} diff --git a/src/cmd/go/testdata/script/version.txt b/src/cmd/go/testdata/script/version.txt index 0123ac6d53..87cb6befe9 100644 --- a/src/cmd/go/testdata/script/version.txt +++ b/src/cmd/go/testdata/script/version.txt @@ -9,6 +9,12 @@ stderr 'with arguments' ! go version -v stderr 'with arguments' +# Neither of the two flags above should be an issue via GOFLAGS. +env GOFLAGS='-m -v' +go version +stdout '^go version' +env GOFLAGS= + env GO111MODULE=on # Skip the builds below if we are running in short mode. [short] skip -- cgit v1.2.3-54-g00ecf