cmd/go: relax version's error on unexpected flags

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 #41264.

Change-Id: I74003dd25d94bacf9ac507b5cad778fd65233321
Reviewed-on: https://go-review.googlesource.com/c/go/+/254157
Trust: Daniel Martí <mvdan@mvdan.cc>
Run-TryBot: Daniel Martí <mvdan@mvdan.cc>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Bryan C. Mills <bcmills@google.com>

diff --git a/src/cmd/go/internal/base/goflags.go b/src/cmd/go/internal/base/goflags.go
index f11f9a5d3365b29198f9a96342b5389b185f566a..4da27550fd3a4f9422a7fceac3adb1b688e83562 100644 (file)
--- a/src/cmd/go/internal/base/goflags.go
+++ b/src/cmd/go/internal/base/goflags.go
@@ -130,3 +130,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 c2de8d326d90e5b6c766878d1164798753b51dbc..5aa0f8e7ed855cea5c3d157b630f363066e60f35 100644 (file)
--- a/src/cmd/go/internal/version/version.go
+++ b/src/cmd/go/internal/version/version.go
@@ -54,7 +54,14 @@ var (
 
 func runVersion(ctx context.Context, 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 f78020032cd74c67ee86c3032c7635ad3c655826..d71387d32363bf83815d5f6b0a3092668a0392a4 100644 (file)
--- a/src/cmd/go/internal/work/init.go
+++ b/src/cmd/go/internal/work/init.go
@@ -254,34 +254,18 @@ func buildModeInit() {
        case "":
                // Behavior will be determined automatically, as if no flag were passed.
        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 81ca6986208e9020a54a0ab0930c26b3442ea292..8615a4aac5978bdf00d8b4c3022a54567cf0cb06 100644 (file)
--- 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