From 5113f5776620dfd0221bb876b9889c73d585371c Mon Sep 17 00:00:00 2001 From: Dan Scales Date: Wed, 11 Mar 2020 12:08:03 -0700 Subject: [PATCH] cmd/go: define a build tag for any GOEXPERIMENT which is enabled. For each experiment that has been enabled in the toolchain, define a build tag with the same name (but prefixed by "goexperiment.") which can be used for compiling alternative files for the experiment. This allows changes for the experiment, like extra struct fields in the runtime, without affecting the base non-experiment code at all. I use this capability in my CL for static lock ranking (https://go-review.googlesource.com/c/go/+/207619), so that static lock ranking can be fully enabled as a GOEXPERIMENT, but there is no overhead in the runtime when the experiment is not enabled. I added a test in cmd/go/testdata/scripts to make sure the build tags are being defined properly. In order to implement the test, I needed to provide environment variable GOEXPSTRING to the test scripts (with its value set from objabi.Expstring(), so that it can determine the experiments baked into the toolchain. I filed https://github.com/golang/go/issues/37937 to make a builder with GOEXPERIMENT set to 'staticlockranking'. This builder will ensure another variant of GOEXPERIMENT is being tested regularly for this change, as well as checking static lock ranking in the runtime. Change-Id: Ieb4b86107238febd105558c1e639d30cfe57ab5c Reviewed-on: https://go-review.googlesource.com/c/go/+/222925 Run-TryBot: Dan Scales TryBot-Result: Gobot Gobot Reviewed-by: Jay Conrod Reviewed-by: Bryan C. Mills --- src/cmd/go/internal/work/init.go | 15 +++ src/cmd/go/script_test.go | 2 + src/cmd/go/testdata/script/README | 1 + .../script/build_tag_goexperiment.txt | 104 ++++++++++++++++++ 4 files changed, 122 insertions(+) create mode 100644 src/cmd/go/testdata/script/build_tag_goexperiment.txt diff --git a/src/cmd/go/internal/work/init.go b/src/cmd/go/internal/work/init.go index e970272954..a574924c5b 100644 --- a/src/cmd/go/internal/work/init.go +++ b/src/cmd/go/internal/work/init.go @@ -10,6 +10,7 @@ import ( "cmd/go/internal/base" "cmd/go/internal/cfg" "cmd/go/internal/load" + "cmd/internal/objabi" "cmd/internal/sys" "flag" "fmt" @@ -34,6 +35,20 @@ func BuildInit() { } cfg.BuildPkgdir = p } + + // For each experiment that has been enabled in the toolchain, define a + // build tag with the same name but prefixed by "goexperiment." which can be + // used for compiling alternative files for the experiment. This allows + // changes for the experiment, like extra struct fields in the runtime, + // without affecting the base non-experiment code at all. [2:] strips the + // leading "X:" from objabi.Expstring(). + exp := objabi.Expstring()[2:] + if exp != "none" { + experiments := strings.Split(exp, ",") + for _, expt := range experiments { + cfg.BuildContext.BuildTags = append(cfg.BuildContext.BuildTags, "goexperiment."+expt) + } + } } func instrumentInit() { diff --git a/src/cmd/go/script_test.go b/src/cmd/go/script_test.go index 3eb66f9166..87afb6aec8 100644 --- a/src/cmd/go/script_test.go +++ b/src/cmd/go/script_test.go @@ -30,6 +30,7 @@ import ( "cmd/go/internal/robustio" "cmd/go/internal/txtar" "cmd/go/internal/work" + "cmd/internal/objabi" "cmd/internal/sys" ) @@ -119,6 +120,7 @@ func (ts *testScript) setup() { "GOCACHE=" + testGOCACHE, "GODEBUG=" + os.Getenv("GODEBUG"), "GOEXE=" + cfg.ExeSuffix, + "GOEXPSTRING=" + objabi.Expstring()[2:], "GOOS=" + runtime.GOOS, "GOPATH=" + filepath.Join(ts.workdir, "gopath"), "GOPROXY=" + proxyURL, diff --git a/src/cmd/go/testdata/script/README b/src/cmd/go/testdata/script/README index 81b6d9d814..f4c92e65ab 100644 --- a/src/cmd/go/testdata/script/README +++ b/src/cmd/go/testdata/script/README @@ -29,6 +29,7 @@ Scripts also have access to these other environment variables: GOARCH= GOCACHE= GOEXE= + GOEXPSTRING= GOOS= GOPATH=$WORK/gopath GOPROXY= diff --git a/src/cmd/go/testdata/script/build_tag_goexperiment.txt b/src/cmd/go/testdata/script/build_tag_goexperiment.txt new file mode 100644 index 0000000000..26ad029845 --- /dev/null +++ b/src/cmd/go/testdata/script/build_tag_goexperiment.txt @@ -0,0 +1,104 @@ +# compile_ext will fail if the buildtags that are enabled (or not enabled) for the +# framepointer and fieldtrack experiments are not consistent with the value of +# GOEXPSTRING (which comes from objabi.Expstring()). + +[short] skip +go run m + +-- expt_main.go -- +package main + +import ( + "os" + "strings" +) + +func main() { + fp() + ft() +} + +func hasExpEntry(s string) bool { + // script_test.go defines GOEXPSTRING to be the value of + // objabi.Expstring(), which gives the enabled experiments baked into the + // toolchain. + g := os.Getenv("GOEXPSTRING") + for _, f := range strings.Split(g, ",") { + if f == s { + return true + } + } + return false +} + +-- fp_off.go -- +// +build !goexperiment.framepointer + +package main + +import ( + "fmt" + "os" +) + +func fp() { + if hasExpEntry("framepointer") { + fmt.Println("in !framepointer build, but objabi.Expstring() has 'framepointer'") + os.Exit(1) + } +} + +-- fp_on.go -- +// +build goexperiment.framepointer + +package main + +import ( + "fmt" + "os" +) + +func fp() { + if !hasExpEntry("framepointer") { + fmt.Println("in framepointer build, but objabi.Expstring() does not have 'framepointer', is", os.Getenv("GOEXPSTRING")) + os.Exit(1) + } +} + +-- ft_off.go -- +// +build !goexperiment.fieldtrack + +package main + +import ( + "fmt" + "os" +) + +func ft() { + if hasExpEntry("fieldtrack") { + fmt.Println("in !fieldtrack build, but objabi.Expstring() has 'fieldtrack'") + os.Exit(1) + } +} + +-- ft_on.go -- +// +build goexperiment.fieldtrack + +package main + +import ( + "fmt" + "os" +) + +func ft() { + if !hasExpEntry("fieldtrack") { + fmt.Println("in fieldtrack build, but objabi.Expstring() does not have 'fieldtrack', is", os.Getenv("GOEXPSTRING")) + os.Exit(1) + } +} + +-- go.mod -- +module m +go 1.14 -- 2.50.0