[go] cmd/go: add $GOFLAGS environment variable
Russ Cox (Gerrit)
Russ Cox would like Rob Pike and Bryan C. Mills to review this change.
cmd/go: add $GOFLAGS environment variable
People sometimes want to turn on a particular go command flag by default.
In Go 1.11 we have at least two different cases where users may need this.
1. Linking can be noticeably slower on underpowered systems
due to DWARF, and users may want to set -ldflags=-w by default.
2. For modules, some users or CI systems will want vendoring always,
so they want -getmode=vendor (soon to be -mod=vendor) by default.
This CL generalizes the problem to “set default flags for the go command.”
$GOFLAGS can be a space-separated list of flag settings, but each
space-separated entry in the list must be a standalone flag.
That is, you must do 'GOFLAGS=-ldflags=-w' not 'GOFLAGS=-ldflags -w'.
The latter would mean to pass -w to go commands that understand it
(if any do; if not, it's an error to mention it).
For #26074.
For #26318.
Fixes #26585.
Change-Id: I428f79c1fbfb9e41e54d199c68746405aed2319c
---
M src/cmd/go/alldocs.go
A src/cmd/go/internal/base/goflags.go
M src/cmd/go/internal/cmdflag/flag.go
M src/cmd/go/internal/envcmd/env.go
M src/cmd/go/internal/help/helpdoc.go
M src/cmd/go/internal/test/testflag.go
M src/cmd/go/internal/vet/vetflag.go
M src/cmd/go/main.go
A src/cmd/go/testdata/script/goflags.txt
M src/make.bash
M src/make.bat
M src/make.rc
12 files changed, 198 insertions(+), 1 deletion(-)
diff --git a/src/cmd/go/alldocs.go b/src/cmd/go/alldocs.go
index 5d7dea8..242e00f 100644
--- a/src/cmd/go/alldocs.go
+++ b/src/cmd/go/alldocs.go
@@ -1394,6 +1394,10 @@
// GOCACHE
// The directory where the go command will store cached
// information for reuse in future builds.
+// GOFLAGS
+// A space-separated list of -flag=value settings to apply
+// to go commands by default (when the given flag is known by
+// the current command).
// GOOS
// The operating system for which to compile code.
// Examples are linux, darwin, windows, netbsd.
diff --git a/src/cmd/go/internal/base/goflags.go b/src/cmd/go/internal/base/goflags.go
new file mode 100644
index 0000000..46a38e2
--- /dev/null
+++ b/src/cmd/go/internal/base/goflags.go
@@ -0,0 +1,134 @@
+// Copyright 2018 The Go Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style
+// license that can be found in the LICENSE file.
+
+package base
+
+import (
+ "cmd/go/internal/cfg"
+ "flag"
+ "fmt"
+ "os"
+ "strings"
+)
+
+var (
+ goflags []string
+ knownFlag = make(map[string]bool)
+)
+
+// AddKnownFlag adds name to the list of known flags for use in GOFLAGS.
+func AddKnownFlag(name string) {
+ knownFlag[name] = true
+}
+
+// GOFLAGS returns the flags from GOFLAGS.
+// The list can be assumed to contain one string per flag,
+// with each string either beginning with -name or --name.
+func GOFLAGS() []string {
+ InitGOFLAGS()
+ return goflags
+}
+
+// InitGOFLAGS initializes the goflags list from from the GOFLAGS environment variable.
+// It is idempotent.
+func InitGOFLAGS() {
+ if goflags != nil { // already initialized
+ return
+ }
+
+ goflags = strings.Fields(os.Getenv("GOFLAGS"))
+ if goflags == nil {
+ goflags = []string{}
+ }
+
+ // Build list of all flags for all commands.
+ // If no command has that flag, then we report the problem.
+ // This catches typos while still letting users record flags in GOFLAGS
+ // that only apply to a subset of go commands.
+ // Commands using CustomFlags can report their flag names
+ // by calling AddKnownFlag instead.
+ var walkFlags func(*Command)
+ walkFlags = func(cmd *Command) {
+ for _, sub := range cmd.Commands {
+ walkFlags(sub)
+ }
+ cmd.Flag.VisitAll(func(f *flag.Flag) {
+ knownFlag[f.Name] = true
+ })
+ }
+ walkFlags(Go)
+
+ // Ignore bad flag in go env and go bug, because
+ // they are what people reach for when debugging
+ // a problem, and maybe they're debugging GOFLAGS.
+ // (Both will show the GOFLAGS setting if let succeed.)
+ hideErrors := cfg.CmdName == "env" || cfg.CmdName == "bug"
+
+ // Each of the words returned by strings.Fields must be its own flag.
+ // (To set flag arguments use -x=value instead of -x value.)
+ for _, f := range goflags {
+ if !strings.HasPrefix(f, "-") || f == "-" || f == "--" || strings.HasPrefix(f, "---") {
+ if hideErrors {
+ continue
+ }
+ Fatalf("go: parsing $GOFLAGS: non-flag %q", f)
+ }
+
+ name := f[1:]
+ if i := strings.Index(name, "="); i >= 0 {
+ name = name[:i]
+ }
+ if !knownFlag[name] {
+ if hideErrors {
+ continue
+ }
+ Fatalf("go: parsing $GOFLAGS: unknown flag -%s", name)
+ }
+ }
+}
+
+type boolFlag interface {
+ flag.Value
+ IsBoolFlag() bool
+}
+
+// SetFromGOFLAGS sets the flags in the given flag set using settings in $GOFLAGS.
+func SetFromGOFLAGS(flags flag.FlagSet) {
+ InitGOFLAGS()
+ for _, goflag := range goflags {
+ name, value, hasValue := goflag, "", false
+ if i := strings.Index(goflag, "="); i >= 0 {
+ name, value, hasValue = goflag[:i], goflag[i+1:], true
+ }
+ if strings.HasPrefix(name, "--") {
+ name = name[1:]
+ }
+ f := flags.Lookup(name[1:])
+ if f == nil {
+ continue
+ }
+ if fb, ok := f.Value.(boolFlag); ok && fb.IsBoolFlag() {
+ if hasValue {
+ if err := fb.Set(value); err != nil {
+ fmt.Fprintf(flags.Output(), "invalid boolean value %q for flag -%s (from $GOFLAGS): %v", value, name, err)
+ flags.Usage()
+ }
+ } else {
+ if err := fb.Set("true"); err != nil {
+ fmt.Fprintf(flags.Output(), "invalid boolean flag -%s (from $GOFLAGS): %v", name, err)
+ flags.Usage()
+ }
+ }
+ } else {
+ if !hasValue {
+ fmt.Fprintf(flags.Output(), "flag needs an argument: -%s (from $GOFLAGS)", name)
+ flags.Usage()
+ }
+ if err := f.Value.Set(value); err != nil {
+ fmt.Fprintf(flags.Output(), "invalid value %q for flag -%s (from $GOFLAGS): %v", value, name, err)
+ flags.Usage()
+ }
+ }
+ }
+}
diff --git a/src/cmd/go/internal/cmdflag/flag.go b/src/cmd/go/internal/cmdflag/flag.go
index 7ab3022..88826fb 100644
--- a/src/cmd/go/internal/cmdflag/flag.go
+++ b/src/cmd/go/internal/cmdflag/flag.go
@@ -121,3 +121,23 @@
f = nil
return
}
+
+func FindGOFLAGS(defns []*Defn) []string {
+ var flags []string
+ for _, flag := range base.GOFLAGS() {
+ if strings.HasPrefix(flag, "--") {
+ flag = flag[1:]
+ }
+ name := flag[1:]
+ if i := strings.Index(name, "="); i >= 0 {
+ name = name[:i]
+ }
+ for _, f := range defns {
+ if name == f.Name {
+ flags = append(flags, flag)
+ break
+ }
+ }
+ }
+ return flags
+}
diff --git a/src/cmd/go/internal/envcmd/env.go b/src/cmd/go/internal/envcmd/env.go
index 1f45848..afadbad 100644
--- a/src/cmd/go/internal/envcmd/env.go
+++ b/src/cmd/go/internal/envcmd/env.go
@@ -54,6 +54,7 @@
{Name: "GOBIN", Value: cfg.GOBIN},
{Name: "GOCACHE", Value: cache.DefaultDir()},
{Name: "GOEXE", Value: cfg.ExeSuffix},
+ {Name: "GOFLAGS", Value: os.Getenv("GOFLAGS")},
{Name: "GOHOSTARCH", Value: runtime.GOARCH},
{Name: "GOHOSTOS", Value: runtime.GOOS},
{Name: "GOOS", Value: cfg.Goos},
diff --git a/src/cmd/go/internal/help/helpdoc.go b/src/cmd/go/internal/help/helpdoc.go
index b5fab2f..3cb39f3 100644
--- a/src/cmd/go/internal/help/helpdoc.go
+++ b/src/cmd/go/internal/help/helpdoc.go
@@ -486,6 +486,10 @@
GOCACHE
The directory where the go command will store cached
information for reuse in future builds.
+ GOFLAGS
+ A space-separated list of -flag=value settings to apply
+ to go commands by default (when the given flag is known by
+ the current command).
GOOS
The operating system for which to compile code.
Examples are linux, darwin, windows, netbsd.
diff --git a/src/cmd/go/internal/test/testflag.go b/src/cmd/go/internal/test/testflag.go
index 8a686b7..4621dfe 100644
--- a/src/cmd/go/internal/test/testflag.go
+++ b/src/cmd/go/internal/test/testflag.go
@@ -63,6 +63,11 @@
// add build flags to testFlagDefn
func init() {
+ for _, f := range testFlagDefn {
+ base.AddKnownFlag(f.Name)
+ base.AddKnownFlag("test." + f.Name)
+ }
+
var cmd base.Command
work.AddBuildFlags(&cmd)
cmd.Flag.VisitAll(func(f *flag.Flag) {
@@ -87,6 +92,7 @@
// go test fmt -custom-flag-for-fmt-test
// go test -x math
func testFlags(args []string) (packageNames, passToTest []string) {
+ args = str.StringList(cmdflag.FindGOFLAGS(testFlagDefn), args)
inPkg := false
var explicitArgs []string
for i := 0; i < len(args); i++ {
diff --git a/src/cmd/go/internal/vet/vetflag.go b/src/cmd/go/internal/vet/vetflag.go
index bdfe033..bdde1bb 100644
--- a/src/cmd/go/internal/vet/vetflag.go
+++ b/src/cmd/go/internal/vet/vetflag.go
@@ -12,6 +12,7 @@
"cmd/go/internal/base"
"cmd/go/internal/cmdflag"
+ "cmd/go/internal/str"
"cmd/go/internal/work"
)
@@ -59,6 +60,10 @@
// add build flags to vetFlagDefn.
func init() {
+ for _, f := range vetFlagDefn {
+ base.AddKnownFlag(f.Name)
+ base.AddKnownFlag("vet." + f.Name)
+ }
var cmd base.Command
work.AddBuildFlags(&cmd)
cmd.Flag.StringVar(&vetTool, "vettool", "", "path to vet tool binary") // for cmd/vet tests; undocumented for now
@@ -73,6 +78,7 @@
// vetFlags processes the command line, splitting it at the first non-flag
// into the list of flags and list of packages.
func vetFlags(args []string) (passToVet, packageNames []string) {
+ args = str.StringList(cmdflag.FindGOFLAGS(vetFlagDefn), args)
for i := 0; i < len(args); i++ {
if !strings.HasPrefix(args[i], "-") {
return args[:i], args[i:]
diff --git a/src/cmd/go/main.go b/src/cmd/go/main.go
index 0743b99..25dfe8f 100644
--- a/src/cmd/go/main.go
+++ b/src/cmd/go/main.go
@@ -209,6 +209,7 @@
if cmd.CustomFlags {
args = args[1:]
} else {
+ base.SetFromGOFLAGS(cmd.Flag)
cmd.Flag.Parse(args[1:])
args = cmd.Flag.Args()
}
diff --git a/src/cmd/go/testdata/script/goflags.txt b/src/cmd/go/testdata/script/goflags.txt
new file mode 100644
index 0000000..c1b8315
--- /dev/null
+++ b/src/cmd/go/testdata/script/goflags.txt
@@ -0,0 +1,19 @@
+# GOFLAGS sets flags for commands
+
+env GOFLAGS='-e -f={{.Dir}} -test.benchtime=1s -count=10'
+go list asdfasdfasdf # succeeds because of -e
+go list runtime
+stdout '[\\/]runtime$'
+
+# go env succeeds even though -f={{.Dir}} is inappropriate
+go env
+
+# bad flags are diagnosed
+env GOFLAGS=-typoflag
+! go list runtime
+stderr 'unknown flag -typoflag'
+
+# except in go bug (untested) and go env
+go env
+stdout GOFLAGS
+
diff --git a/src/make.bash b/src/make.bash
index a28b82a..78882d9 100755
--- a/src/make.bash
+++ b/src/make.bash
@@ -63,6 +63,7 @@
set -e
unset GOBIN # Issue 14340
+unset GOFLAGS
if [ ! -f run.bash ]; then
echo 'make.bash must be run from $GOROOT/src' 1>&2
diff --git a/src/make.bat b/src/make.bat
index 9df49cd..2e71833 100644
--- a/src/make.bat
+++ b/src/make.bat
@@ -47,6 +47,7 @@
:nolocal
set GOBUILDFAIL=0
+set GOFLAGS=
if exist make.bat goto ok
echo Must run make.bat from Go src directory.
diff --git a/src/make.rc b/src/make.rc
index 7ae6221..a97dfc8 100755
--- a/src/make.rc
+++ b/src/make.rc
@@ -47,7 +47,7 @@
shift
}
-
+GOFLAGS=()
GOROOT = `{cd .. && pwd}
if(! ~ $#GOROOT_BOOTSTRAP 1)
GOROOT_BOOTSTRAP = $home/go1.4
To view, visit change 126656. To unsubscribe, or for help writing mail filters, visit settings.
Gobot Gobot (Gerrit)
TryBots beginning. Status page: https://farmer.golang.org/try?commit=a4ab2a46
Gobot Gobot (Gerrit)
TryBots are happy.
Patch set 1:TryBot-Result +1
Rob Pike (Gerrit)
9 comments:
File src/cmd/go/internal/base/goflags.go:
Patch Set #1, Line 34:
// It is idempotent.not exactly. it does nothing the second time, which isn't quite the same as idempotent.
// If goflags is already initialized, it does nothing.
Patch Set #1, Line 69:
// (To set flag arguments use -x=value instead of -x value.)drop the parens. also clarify that -boolean is ok without -boolean=true, i think.
Patch Set #1, Line 71:
if !strings.HasPrefix(f, "-") || f == "-" || f == "--" || strings.HasPrefix(f, "---") {what on earth is triple-minus?
Patch Set #1, Line 86:
Fatalf("go: parsing $GOFLAGS: unknown flag -%s", name)maybe use %q and drop the -?
also an empty flag name might deserve its own error message.Patch Set #1, Line 93:
IsBoolFlag() boolmention that this is how the flag package does this.
(IsBoolFlag interface could well be exported by the flag package. not sure why it's not.)
File src/cmd/go/internal/cmdflag/flag.go:
Patch Set #1, Line 131:
name := flag[1:]check that name is not empty. oh but then what does `GOFLAGS=--` mean?
File src/cmd/go/internal/test/testflag.go:
Patch Set #1, Line 66:
for _, f := range testFlagDefn {this thunderclap could be a method on base.
File src/cmd/go/internal/vet/vetflag.go:
Patch Set #1, Line 63:
for _, f := range vetFlagDefn {see comment in testflag.go.
File src/cmd/go/testdata/script/goflags.txt:
Patch Set #1, Line 5:
go list runtimetrailing space
To view, visit change 126656. To unsubscribe, or for help writing mail filters, visit settings.
Ralph Corderoy (Gerrit)
9 comments:
File src/cmd/go/internal/base/goflags.go:
Patch Set #1, Line 16:
goflags []stringComment that they have their leading '-' or '--'.
Patch Set #1, Line 17:
knownFlag = make(map[string]bool)Comment that they do not have their leading '-' or '--'.
Patch Set #1, Line 25:
// GOFLAGS returns the flags from GOFLAGS.s/GOFLAGS/the & environment variable/2
Or $GOFLAGS, having seen below.Patch Set #1, Line 33:
// InitGOFLAGS initializes the goflags list from from the GOFLAGS environment variable.s/from from/from
goflags = strings.Fields(os.Getenv("GOFLAGS"))
if goflags == nil {
goflags = []string{}
}The next two blocks don't use goflags.
Perhaps move it to after them so it's near its usage.
Patch Set #1, Line 71:
if !strings.HasPrefix(f, "-") || f == "-" || f == "--" || strings.HasPrefix(f, "---") {what on earth is triple-minus?
I think he's ruling out --- so what's left must be /^--?[^-]/
Patch Set #1, Line 78:
name := f[1:]Doesn't this need to also skip a second optional '-'?
I think knownFlag's keys are without leading '-' or '--'.Patch Set #1, Line 96:
// SetFromGOFLAGS sets the flags in the given flag set using settings in $GOFLAGS.$GOFLAGS here, 'the GOFLAGS environment variable' elsewhere.
Perhaps it's normal to go for brevity after the earlier occurrences.
File src/cmd/go/testdata/script/goflags.txt:
Patch Set #1, Line 3:
env GOFLAGS='-e -f={{.Dir}} -test.benchtime=1s -count=10'Test a -- flag.
To view, visit change 126656. To unsubscribe, or for help writing mail filters, visit settings.
David Chase (Gerrit)
Should run.bash check for non-empty GOFLAGS and warn about it?
There might be cases where we want to set it, but setting it by accident will at best lead to wasted testing runs (use case: "./run.bash", go get coffee, come back to see that it failed midway through because of GOFLAGS).
1 comment:
File src/cmd/go/internal/base/goflags.go:
Patch Set #1, Line 86:
Fatalf("go: parsing $GOFLAGS: unknown flag -%s", name)maybe use %q and drop the -? […]
Empty flag is caught above with test on line 71, report on line 75.
To view, visit change 126656. To unsubscribe, or for help writing mail filters, visit settings.
Than McIntosh (Gerrit)
Is it worth documenting that the flags in $GOFLAGS are logically prepended to the command line as opposed to being appended? That is, if $GOFLAGS is set to -ldflags=-w, you can override that behavior by doing "-ldflags="? This is easily discoverable, but perhaps it would help for users to know about it up front.
Bryan C. Mills (Gerrit)
Patch set 1:Code-Review +1
5 comments:
File src/cmd/go/internal/base/goflags.go:
Patch Set #1, Line 99:
for _, goflag := range goflags {A comment would be helpful here. How does this loop differ from calling `flags.Parse(goflags)`?
(It looks like at least part of this loop was copied verbatim from flags.Parse.)
File src/cmd/go/internal/cmdflag/flag.go:
Patch Set #1, Line 125:
func FindGOFLAGS(defns []*Defn) []string {A comment or named return-value for this function would be helpful. (It seems to be returning a filtered subset of base.GOFLAGS(), but it took me a bit of reading to figure that out.)
Patch Set #1, Line 131:
name := flag[1:]
check that name is not empty. […]
Both of those are disallowed by the doc comment for base.GOFLAGS and explicitly rejected at goflags.go:71.
File src/cmd/go/testdata/script/goflags.txt:
Patch Set #1, Line 1:
# GOFLAGS sets flags for commandsSome build flags (-race,-msan) are supported only for limited GOOS/GOARCH combinations.
Please include a test illustrating how they interact with GOFLAGS.
Patch Set #1, Line 12:
env GOFLAGS=-typoflagAlso explicitly reject:
env GOFLAGS=-=nameless
env GOFLAGS=--
env GOFLAGS=---oops
env GOFLAGS=-And perhaps boolean vs. non-boolean flags?
To view, visit change 126656. To unsubscribe, or for help writing mail filters, visit settings.
Russ Cox (Gerrit)
Russ Cox uploaded patch set #2 to this change.
12 files changed, 231 insertions(+), 1 deletion(-)
To view, visit change 126656. To unsubscribe, or for help writing mail filters, visit settings.
Russ Cox (Gerrit)
Uploaded patch set 2.
(4 comments)
4 comments:
Patch Set #1, Line 1:
# GOFLAGS sets flags for commands
Some build flags (-race,-msan) are supported only for limited GOOS/GOARCH combinations. […]
Done although there's nothing special about it.
The flag is always *defined* or not regardless of GOOS/GOARCH.
If the flag is defined, it gets copied from GOFLAGS.*Using* the flag is not supported in those GOOS/GOARCH combinations.
That's different.Patch Set #1, Line 3:
env GOFLAGS='-e -f={{.Dir}} -test.benchtime=1s -count=10'Test a -- flag.
Done
Patch Set #1, Line 5:
go list runtimetrailing space
Done
Patch Set #1, Line 12:
env GOARCH=$OLDGOARCH GOOS=$OLDGOOSAlso explicitly reject: […]
Done
To view, visit change 126656. To unsubscribe, or for help writing mail filters, visit settings.
Gobot Gobot (Gerrit)
Russ Cox (Gerrit)
Should run.bash check for non-empty GOFLAGS and warn about it?
Cleared instead, like in make.bash.
I'm not signing up for making run.bash arbitrary-GOFLAGS-safe. :-)
Is it worth documenting that the flags in $GOFLAGS are logically prepended...
Done.
16 comments:
File src/cmd/go/internal/base/goflags.go:
Patch Set #1, Line 16:
goflags []string
Comment that they have their leading '-' or '--'.
Done
Patch Set #1, Line 17:
knownFlag = make(map[string]bool)
Comment that they do not have their leading '-' or '--'.
Done
Patch Set #1, Line 25:
// GOFLAGS returns the flags from GOFLAGS.s/GOFLAGS/the & environment variable/2 […]
Done
Patch Set #1, Line 33:
// InitGOFLAGS initializes the goflags list from from the GOFLAGS environment variable.s/from from/from
Done
Patch Set #1, Line 34:
// It is idempotent.not exactly. it does nothing the second time, which isn't quite the same as idempotent. […]
Done
goflags = strings.Fields(os.Getenv("GOFLAGS"))
if goflags == nil {
goflags = []string{}
}
The next two blocks don't use goflags. […]
Done
Patch Set #1, Line 69:
// (To set flag arguments use -x=value instead of -x value.)
drop the parens. also clarify that -boolean is ok without -boolean=true, i think.
Done
Patch Set #1, Line 71:
if !strings.HasPrefix(f, "-") || f == "-" || f == "--" || strings.HasPrefix(f, "---") || strings.HasPrefix(f, "-=") || strings.HasPrefix(f, "--=") {
I think he's ruling out --- so what's left must be /^--?[^-]/
Yes. Commented.
Patch Set #1, Line 78:
name := f[1:]
Doesn't this need to also skip a second optional '-'? […]
Yes, thanks.
Patch Set #1, Line 86:
if hideErrors {
Empty flag is caught above with test on line 71, report on line 75.
Yes, by now we know the name is mostly OK, and I think unknown flag -foo is clearer than unknown flag "foo".
mention that this is how the flag package does this. […]
Done
Patch Set #1, Line 96:
IsBoolFlag() bool$GOFLAGS here, 'the GOFLAGS environment variable' elsewhere. […]
I try to avoid saying $GOFLAGS in public docs because on Windows it's %GOFLAGS%.
So instead I say "GOFLAGS environment variable" or just GOFLAGS if that's clear.
But in code comments $GOFLAGS is fine. Fixed.Patch Set #1, Line 99:
// SetFromGOFLAGS sets the flags in the given flag set using settings in $GOFLAGS.A comment would be helpful here. How does this loop differ from calling `flags.Parse(goflags)`? […]
Done
File src/cmd/go/internal/cmdflag/flag.go:
Patch Set #1, Line 125:
func FindGOFLAGS(defns []*Defn) []string {
A comment or named return-value for this function would be helpful. […]
Done
File src/cmd/go/internal/test/testflag.go:
Patch Set #1, Line 66:
for _, f := range testFlagDefn {this thunderclap could be a method on base.
Done (in cmdflag).
File src/cmd/go/internal/vet/vetflag.go:
Patch Set #1, Line 63:
for _, f := range vetFlagDefn {see comment in testflag.go.
Done
To view, visit change 126656. To unsubscribe, or for help writing mail filters, visit settings.
Gobot Gobot (Gerrit)
TryBots are happy.
Patch set 2:TryBot-Result +1
Russ Cox (Gerrit)
Uploaded patch set 3.
Russ Cox (Gerrit)
Russ Cox uploaded patch set #3 to this change.
M src/run.bash
M src/run.bat
M src/run.rc
15 files changed, 259 insertions(+), 1 deletion(-)
To view, visit change 126656. To unsubscribe, or for help writing mail filters, visit settings.
Gobot Gobot (Gerrit)
Gobot Gobot (Gerrit)
TryBots are happy.
Patch set 3:TryBot-Result +1
Russ Cox (Gerrit)
PTAL
Rob Pike (Gerrit)
Patch set 3:Code-Review +2
Russ Cox (Gerrit)
Russ Cox merged this change.
cmd/go: add $GOFLAGS environment variable
People sometimes want to turn on a particular go command flag by default.
In Go 1.11 we have at least two different cases where users may need this.
1. Linking can be noticeably slower on underpowered systems
due to DWARF, and users may want to set -ldflags=-w by default.
2. For modules, some users or CI systems will want vendoring always,
so they want -getmode=vendor (soon to be -mod=vendor) by default.
This CL generalizes the problem to “set default flags for the go command.”
$GOFLAGS can be a space-separated list of flag settings, but each
space-separated entry in the list must be a standalone flag.
That is, you must do 'GOFLAGS=-ldflags=-w' not 'GOFLAGS=-ldflags -w'.
The latter would mean to pass -w to go commands that understand it
(if any do; if not, it's an error to mention it).
For #26074.
For #26318.
Fixes #26585.
Change-Id: I428f79c1fbfb9e41e54d199c68746405aed2319c
Reviewed-on: https://go-review.googlesource.com/126656
Run-TryBot: Russ Cox <r...@golang.org>
TryBot-Result: Gobot Gobot <go...@golang.org>
Reviewed-by: Rob Pike <r...@golang.org>
---
M src/cmd/go/alldocs.go
A src/cmd/go/internal/base/goflags.go
M src/cmd/go/internal/cmdflag/flag.go
M src/cmd/go/internal/envcmd/env.go
M src/cmd/go/internal/help/helpdoc.go
M src/cmd/go/internal/test/testflag.go
M src/cmd/go/internal/vet/vetflag.go
M src/cmd/go/main.go
A src/cmd/go/testdata/script/goflags.txt
M src/make.bash
M src/make.bat
M src/make.rc
M src/run.bash
M src/run.bat
M src/run.rc
15 files changed, 259 insertions(+), 1 deletion(-)
diff --git a/src/cmd/go/alldocs.go b/src/cmd/go/alldocs.go
index 5d7dea8..242e00f 100644
--- a/src/cmd/go/alldocs.go
+++ b/src/cmd/go/alldocs.go
@@ -1394,6 +1394,10 @@
// GOCACHE
// The directory where the go command will store cached
// information for reuse in future builds.
+// GOFLAGS
+// A space-separated list of -flag=value settings to apply
+// to go commands by default (when the given flag is known by
+// the current command).
// GOOS
// The operating system for which to compile code.
// Examples are linux, darwin, windows, netbsd.
diff --git a/src/cmd/go/internal/base/goflags.go b/src/cmd/go/internal/base/goflags.go
new file mode 100644
index 0000000..2f50b50
--- /dev/null
+++ b/src/cmd/go/internal/base/goflags.go
@@ -0,0 +1,152 @@
+// Copyright 2018 The Go Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style
+// license that can be found in the LICENSE file.
+
+package base
+
+import (
+ "flag"
+ "fmt"
+ "os"
+ "runtime"
+ "strings"
+
+ "cmd/go/internal/cfg"
+)
+
+var (
+ goflags []string // cached $GOFLAGS list; can be -x or --x form
+ knownFlag = make(map[string]bool) // flags allowed to appear in $GOFLAGS; no leading dashes
+)
+
+// AddKnownFlag adds name to the list of known flags for use in $GOFLAGS.
+func AddKnownFlag(name string) {
+ knownFlag[name] = true
+}
+
+// GOFLAGS returns the flags from $GOFLAGS.
+// The list can be assumed to contain one string per flag,
+// with each string either beginning with -name or --name.
+func GOFLAGS() []string {
+ InitGOFLAGS()
+ return goflags
+}
+
+// InitGOFLAGS initializes the goflags list from $GOFLAGS.
+// If goflags is already initialized, it does nothing.
+func InitGOFLAGS() {
+ if goflags != nil { // already initialized
+ return
+ }
+
+ // Build list of all flags for all commands.
+ // If no command has that flag, then we report the problem.
+ // This catches typos while still letting users record flags in GOFLAGS
+ // that only apply to a subset of go commands.
+ // Commands using CustomFlags can report their flag names
+ // by calling AddKnownFlag instead.
+ var walkFlags func(*Command)
+ walkFlags = func(cmd *Command) {
+ for _, sub := range cmd.Commands {
+ walkFlags(sub)
+ }
+ cmd.Flag.VisitAll(func(f *flag.Flag) {
+ knownFlag[f.Name] = true
+ })
+ }
+ walkFlags(Go)
+
+ // Ignore bad flag in go env and go bug, because
+ // they are what people reach for when debugging
+ // a problem, and maybe they're debugging GOFLAGS.
+ // (Both will show the GOFLAGS setting if let succeed.)
+ hideErrors := cfg.CmdName == "env" || cfg.CmdName == "bug"
+
+ goflags = strings.Fields(os.Getenv("GOFLAGS"))
+ if goflags == nil {
+ goflags = []string{} // avoid work on later InitGOFLAGS call
+ }
+
+ // Each of the words returned by strings.Fields must be its own flag.
+ // To set flag arguments use -x=value instead of -x value.
+ // For boolean flags, -x is fine instead of -x=true.
+ for _, f := range goflags {
+ // Check that every flag looks like -x --x -x=value or --x=value.
+ if !strings.HasPrefix(f, "-") || f == "-" || f == "--" || strings.HasPrefix(f, "---") || strings.HasPrefix(f, "-=") || strings.HasPrefix(f, "--=") {
+ if hideErrors {
+ continue
+ }
+ Fatalf("go: parsing $GOFLAGS: non-flag %q", f)
+ }
+
+ name := f[1:]
+ if name[0] == '-' {
+ name = name[1:]
+ }
+ if i := strings.Index(name, "="); i >= 0 {
+ name = name[:i]
+ }
+ if !knownFlag[name] {
+ if hideErrors {
+ continue
+ }
+ Fatalf("go: parsing $GOFLAGS: unknown flag -%s", name)
+ }
+ }
+}
+
+// boolFlag is the optional interface for flag.Value known to the flag package.
+// (It is not clear why package flag does not export this interface.)
+type boolFlag interface {
+ flag.Value
+ IsBoolFlag() bool
+}
+
+// SetFromGOFLAGS sets the flags in the given flag set using settings in $GOFLAGS.
+func SetFromGOFLAGS(flags flag.FlagSet) {
+ InitGOFLAGS()
+
+ // This loop is similar to flag.Parse except that it ignores
+ // unknown flags found in goflags, so that setting, say, GOFLAGS=-ldflags=-w
+ // does not break commands that don't have a -ldflags.
+ // It also adjusts the output to be clear that the reported problem is from $GOFLAGS.
+ where := "$GOFLAGS"
+ if runtime.GOOS == "windows" {
+ where = "%GOFLAGS%"
+ }
+ for _, goflag := range goflags {
+ name, value, hasValue := goflag, "", false
+ if i := strings.Index(goflag, "="); i >= 0 {
+ name, value, hasValue = goflag[:i], goflag[i+1:], true
+ }
+ if strings.HasPrefix(name, "--") {
+ name = name[1:]
+ }
+ f := flags.Lookup(name[1:])
+ if f == nil {
+ continue
+ }
+ if fb, ok := f.Value.(boolFlag); ok && fb.IsBoolFlag() {
+ if hasValue {
+ if err := fb.Set(value); err != nil {
+ fmt.Fprintf(flags.Output(), "go: invalid boolean value %q for flag %s (from %s): %v\n", value, name, where, err)
+ flags.Usage()
+ }
+ } else {
+ if err := fb.Set("true"); err != nil {
+ fmt.Fprintf(flags.Output(), "go: invalid boolean flag %s (from %s): %v\n", name, where, err)
+ flags.Usage()
+ }
+ }
+ } else {
+ if !hasValue {
+ fmt.Fprintf(flags.Output(), "go: flag needs an argument: %s (from %s)\n", name, where)
+ flags.Usage()
+ }
+ if err := f.Value.Set(value); err != nil {
+ fmt.Fprintf(flags.Output(), "go: invalid value %q for flag %s (from %s): %v\n", value, name, where, err)
+ flags.Usage()
+ }
+ }
+ }
+}
diff --git a/src/cmd/go/internal/cmdflag/flag.go b/src/cmd/go/internal/cmdflag/flag.go
index 7ab3022..b2a67e6 100644
--- a/src/cmd/go/internal/cmdflag/flag.go
+++ b/src/cmd/go/internal/cmdflag/flag.go
@@ -69,6 +69,14 @@
os.Exit(2)
}
+// AddKnownFlags registers the flags in defns with base.AddKnownFlag.
+func AddKnownFlags(cmd string, defns []*Defn) {
+ for _, f := range defns {
+ base.AddKnownFlag(f.Name)
+ base.AddKnownFlag(cmd + "." + f.Name)
+ }
+}
+
// Parse sees if argument i is present in the definitions and if so,
// returns its definition, value, and whether it consumed an extra word.
// If the flag begins (cmd+".") it is ignored for the purpose of this function.
@@ -121,3 +129,31 @@
f = nil
return
}
+
+// FindGOFLAGS extracts and returns the flags matching defns from GOFLAGS.
+// Ideally the caller would mention that the flags were from GOFLAGS
+// when reporting errors, but that's too hard for now.
+func FindGOFLAGS(defns []*Defn) []string {
+ var flags []string
+ for _, flag := range base.GOFLAGS() {
+ // Flags returned by base.GOFLAGS are well-formed, one of:
+ // -x
+ // --x
+ // -x=value
+ // --x=value
index b5fab2f..1798120 100644
--- a/src/cmd/go/internal/help/helpdoc.go
+++ b/src/cmd/go/internal/help/helpdoc.go
@@ -486,6 +486,11 @@
GOCACHE
The directory where the go command will store cached
information for reuse in future builds.
+ GOFLAGS
+ A space-separated list of -flag=value settings to apply
+ to go commands by default, when the given flag is known by
+ the current command. Flags listed on the command-line
+ are applied after this list and therefore override it.
GOOS
The operating system for which to compile code.
Examples are linux, darwin, windows, netbsd.
diff --git a/src/cmd/go/internal/test/testflag.go b/src/cmd/go/internal/test/testflag.go
index 8a686b7..73f8c69 100644
--- a/src/cmd/go/internal/test/testflag.go
+++ b/src/cmd/go/internal/test/testflag.go
@@ -63,6 +63,7 @@
// add build flags to testFlagDefn
func init() {
+ cmdflag.AddKnownFlags("test", testFlagDefn)
var cmd base.Command
work.AddBuildFlags(&cmd)
cmd.Flag.VisitAll(func(f *flag.Flag) {
@@ -87,6 +88,7 @@
// go test fmt -custom-flag-for-fmt-test
// go test -x math
func testFlags(args []string) (packageNames, passToTest []string) {
+ args = str.StringList(cmdflag.FindGOFLAGS(testFlagDefn), args)
inPkg := false
var explicitArgs []string
for i := 0; i < len(args); i++ {
diff --git a/src/cmd/go/internal/vet/vetflag.go b/src/cmd/go/internal/vet/vetflag.go
index bdfe033..6cf2a8c 100644
--- a/src/cmd/go/internal/vet/vetflag.go
+++ b/src/cmd/go/internal/vet/vetflag.go
@@ -12,6 +12,7 @@
"cmd/go/internal/base"
"cmd/go/internal/cmdflag"
+ "cmd/go/internal/str"
"cmd/go/internal/work"
)
@@ -59,6 +60,7 @@
// add build flags to vetFlagDefn.
func init() {
+ cmdflag.AddKnownFlags("vet", vetFlagDefn)
var cmd base.Command
work.AddBuildFlags(&cmd)
cmd.Flag.StringVar(&vetTool, "vettool", "", "path to vet tool binary") // for cmd/vet tests; undocumented for now
@@ -73,6 +75,7 @@
// vetFlags processes the command line, splitting it at the first non-flag
// into the list of flags and list of packages.
func vetFlags(args []string) (passToVet, packageNames []string) {
+ args = str.StringList(cmdflag.FindGOFLAGS(vetFlagDefn), args)
for i := 0; i < len(args); i++ {
if !strings.HasPrefix(args[i], "-") {
return args[:i], args[i:]
diff --git a/src/cmd/go/main.go b/src/cmd/go/main.go
index 0743b99..25dfe8f 100644
--- a/src/cmd/go/main.go
+++ b/src/cmd/go/main.go
@@ -209,6 +209,7 @@
if cmd.CustomFlags {
args = args[1:]
} else {
+ base.SetFromGOFLAGS(cmd.Flag)
cmd.Flag.Parse(args[1:])
args = cmd.Flag.Args()
}
diff --git a/src/cmd/go/testdata/script/goflags.txt b/src/cmd/go/testdata/script/goflags.txt
new file mode 100644
index 0000000..20de325
--- /dev/null
+++ b/src/cmd/go/testdata/script/goflags.txt
@@ -0,0 +1,49 @@
+# GOFLAGS sets flags for commands
+
+env GOFLAGS='-e -f={{.Dir}} --test.benchtime=1s -count=10'
+go list asdfasdfasdf # succeeds because of -e
+go list runtime
+stdout '[\\/]runtime$'
+
+env GOFLAGS=-race OLDGOARCH=$GOARCH OLDGOOS=$GOOS GOARCH=386 GOOS=linux
+! go list runtime
+stderr 'race is only supported on'
+
+env GOARCH=$OLDGOARCH GOOS=$OLDGOOS
+
+# go env succeeds even though -f={{.Dir}} is inappropriate
+go env
+
+# bad flags are diagnosed
+env GOFLAGS=-typoflag
+! go list runtime
+stderr 'unknown flag -typoflag'
+
+env GOFLAGS=-
+! go list runtime
+stderr '^go: parsing \$GOFLAGS: non-flag "-"'
+
+env GOFLAGS=--
+! go list runtime
+stderr '^go: parsing \$GOFLAGS: non-flag "--"'
+
+env GOFLAGS=---oops
+! go list runtime
+stderr '^go: parsing \$GOFLAGS: non-flag "---oops"'
+
+env GOFLAGS=-=noname
+! go list runtime
+stderr '^go: parsing \$GOFLAGS: non-flag "-=noname"'
+
+env GOFLAGS=-f
+! go list runtime
+stderr '^go: flag needs an argument: -f \(from (\$GOFLAGS|%GOFLAGS%)\)$'
+
+env GOFLAGS=-e=asdf
+! go list runtime
+stderr '^go: invalid boolean value \"asdf\" for flag -e \(from (\$GOFLAGS|%GOFLAGS%)\)'
diff --git a/src/run.bash b/src/run.bash
index 5679f99..c14f4a2 100755
--- a/src/run.bash
+++ b/src/run.bash
@@ -20,6 +20,7 @@
unset CDPATH # in case user has it set
unset GOBIN # Issue 14340
+unset GOFLAGS
export GOHOSTOS
export CC
diff --git a/src/run.bat b/src/run.bat
index 6e42922..0e0c413 100644
--- a/src/run.bat
+++ b/src/run.bat
@@ -17,6 +17,7 @@
set GOPATH=
:: Issue 14340: ignore GOBIN during all.bat.
set GOBIN=
+set GOFLAGS=
rem TODO avoid rebuild if possible
diff --git a/src/run.rc b/src/run.rc
index 88d7791..49d6fd9 100755
--- a/src/run.rc
+++ b/src/run.rc
@@ -10,5 +10,6 @@
GOPATH = () # we disallow local import for non-local packages, if $GOROOT happens
# to be under $GOPATH, then some tests below will fail
GOBIN = () # Issue 14340
+GOFLAGS = ()
exec go tool dist test -rebuild $*
To view, visit change 126656. To unsubscribe, or for help writing mail filters, visit settings.
Ian Lance Taylor (Gerrit)
RELNOTE=yes
Tianon Gravi (Gerrit)
1 comment:
File src/cmd/go/internal/base/goflags.go:
Patch Set #4, Line 65:
goflags = strings.Fields(os.Getenv("GOFLAGS"))Doesn't using `strings.Fields` here mean that we can't (for example) have a value which contains spaces? I often use a `-ldflags` value of `-d -s -w`, which I don't think is possible using this new environment variable, if I'm reading this CL correctly?
(To be clear, this feature is cool and I'm really excited about it, just trying to make sure I understand the limitation since I don't see it discussed explicitly anywhere here or in the three linked issues ♥)
To view, visit change 126656. To unsubscribe, or for help writing mail filters, visit settings.
Ian Lance Taylor (Gerrit)
1 comment:
File src/cmd/go/internal/base/goflags.go:
Patch Set #4, Line 65:
goflags = strings.Fields(os.Getenv("GOFLAGS"))Doesn't using `strings. […]
I think you're right. This should perhaps use str.SplitQuotedFields instead. Can you open an issue, ideally with a simple test case? Thanks.
To view, visit change 126656. To unsubscribe, or for help writing mail filters, visit settings.
Bryan C. Mills (Gerrit)
1 comment:
File src/cmd/go/internal/base/goflags.go:
Patch Set #4, Line 65:
goflags = strings.Fields(os.Getenv("GOFLAGS"))I think you're right. This should perhaps use str.SplitQuotedFields instead. […]
To view, visit change 126656. To unsubscribe, or for help writing mail filters, visit settings.