From dba89283ad05e600defc41dbf94df91ce9027566 Mon Sep 17 00:00:00 2001 From: Russ Cox Date: Thu, 15 Apr 2021 23:07:41 -0400 Subject: [PATCH] cmd/go, go/build: add ToolTags to build.Default The build.Default context really needs to accurately describe the default build context. The goexperiment tags being a special case in the go command violates that rule and is the root cause of the various try-bot failures blocking the enabling of regabi. (The cleanups I made in golang.org/x/tools were long overdue but are not strictly necessary for making regabi work; this CL is.) Having moved the GOEXPERIMENT parsing into internal/buildcfg, go/build can now use it to set up build.Default, in the new field ToolTags, meant to hold toolchain-determined tags (for now, just the experiments). And at the same time we can remove the duplication of GOOS and GOARCH defaults. And then once build.Default is set up accurately, the special case code in cmd/go itself can be removed, and the special case code in test/run.go is at least a bit less special. Change-Id: Ib7394e10aa018e492cb9a83fb8fb9a5011a8c25b Reviewed-on: https://go-review.googlesource.com/c/go/+/310732 Trust: Russ Cox Run-TryBot: Russ Cox Reviewed-by: Jay Conrod Reviewed-by: Austin Clements --- src/cmd/go/alldocs.go | 1 + src/cmd/go/internal/list/context.go | 2 ++ src/cmd/go/internal/list/list.go | 1 + src/cmd/go/internal/work/init.go | 10 --------- src/go/build/build.go | 34 ++++++++++++++++++++++------- test/run.go | 25 +++++---------------- 6 files changed, 35 insertions(+), 38 deletions(-) diff --git a/src/cmd/go/alldocs.go b/src/cmd/go/alldocs.go index 999fcf7e53..9d4626769d 100644 --- a/src/cmd/go/alldocs.go +++ b/src/cmd/go/alldocs.go @@ -841,6 +841,7 @@ // UseAllFiles bool // use files regardless of +build lines, file names // Compiler string // compiler to assume when computing target paths // BuildTags []string // build constraints to match in +build lines +// ToolTags []string // toolchain-specific build constraints // ReleaseTags []string // releases the current release is compatible with // InstallSuffix string // suffix to use in the name of the install dir // } diff --git a/src/cmd/go/internal/list/context.go b/src/cmd/go/internal/list/context.go index 68d691ebe2..2dc63766b7 100644 --- a/src/cmd/go/internal/list/context.go +++ b/src/cmd/go/internal/list/context.go @@ -17,6 +17,7 @@ type Context struct { UseAllFiles bool `json:",omitempty"` // use files regardless of +build lines, file names Compiler string `json:",omitempty"` // compiler to assume when computing target paths BuildTags []string `json:",omitempty"` // build constraints to match in +build lines + ToolTags []string `json:",omitempty"` // toolchain-specific build constraints ReleaseTags []string `json:",omitempty"` // releases the current release is compatible with InstallSuffix string `json:",omitempty"` // suffix to use in the name of the install dir } @@ -31,6 +32,7 @@ func newContext(c *build.Context) *Context { UseAllFiles: c.UseAllFiles, Compiler: c.Compiler, BuildTags: c.BuildTags, + ToolTags: c.ToolTags, ReleaseTags: c.ReleaseTags, InstallSuffix: c.InstallSuffix, } diff --git a/src/cmd/go/internal/list/list.go b/src/cmd/go/internal/list/list.go index 221370bd19..53bf75e27e 100644 --- a/src/cmd/go/internal/list/list.go +++ b/src/cmd/go/internal/list/list.go @@ -148,6 +148,7 @@ The template function "context" returns the build context, defined as: UseAllFiles bool // use files regardless of +build lines, file names Compiler string // compiler to assume when computing target paths BuildTags []string // build constraints to match in +build lines + ToolTags []string // toolchain-specific build constraints ReleaseTags []string // releases the current release is compatible with InstallSuffix string // suffix to use in the name of the install dir } diff --git a/src/cmd/go/internal/work/init.go b/src/cmd/go/internal/work/init.go index 66e1ca7d80..ca7e04d280 100644 --- a/src/cmd/go/internal/work/init.go +++ b/src/cmd/go/internal/work/init.go @@ -14,7 +14,6 @@ import ( "cmd/internal/sys" "flag" "fmt" - "internal/buildcfg" "os" "path/filepath" "runtime" @@ -46,15 +45,6 @@ func BuildInit() { base.Fatalf("go %s: %s environment variable is relative; must be absolute path: %s\n", flag.Args()[0], key, path) } } - - // 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. - for _, expt := range buildcfg.EnabledExperiments() { - cfg.BuildContext.BuildTags = append(cfg.BuildContext.BuildTags, "goexperiment."+expt) - } } func instrumentInit() { diff --git a/src/go/build/build.go b/src/go/build/build.go index 501ce2e8dd..8d1a107c6e 100644 --- a/src/go/build/build.go +++ b/src/go/build/build.go @@ -12,6 +12,7 @@ import ( "go/build/constraint" "go/doc" "go/token" + "internal/buildcfg" exec "internal/execabs" "internal/goroot" "internal/goversion" @@ -48,16 +49,18 @@ type Context struct { UseAllFiles bool // use files regardless of +build lines, file names Compiler string // compiler to assume when computing target paths - // The build and release tags specify build constraints + // The build, tool, and release tags specify build constraints // that should be considered satisfied when processing +build lines. // Clients creating a new context may customize BuildTags, which - // defaults to empty, but it is usually an error to customize ReleaseTags, - // which defaults to the list of Go releases the current release is compatible with. + // defaults to empty, but it is usually an error to customize ToolTags or ReleaseTags. + // ToolTags defaults to build tags appropriate to the current Go toolchain configuration. + // ReleaseTags defaults to the list of Go releases the current release is compatible with. // BuildTags is not set for the Default build Context. - // In addition to the BuildTags and ReleaseTags, build constraints + // In addition to the BuildTags, ToolTags, and ReleaseTags, build constraints // consider the values of GOARCH and GOOS as satisfied tags. // The last element in ReleaseTags is assumed to be the current release. BuildTags []string + ToolTags []string ReleaseTags []string // The install suffix specifies a suffix to use in the name of the installation @@ -292,17 +295,27 @@ func defaultGOPATH() string { return "" } -var defaultReleaseTags []string +var defaultToolTags, defaultReleaseTags []string func defaultContext() Context { var c Context - c.GOARCH = envOr("GOARCH", runtime.GOARCH) - c.GOOS = envOr("GOOS", runtime.GOOS) + c.GOARCH = buildcfg.GOARCH + c.GOOS = buildcfg.GOOS c.GOROOT = pathpkg.Clean(runtime.GOROOT()) c.GOPATH = envOr("GOPATH", defaultGOPATH()) c.Compiler = runtime.Compiler + // 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. + for _, exp := range buildcfg.EnabledExperiments() { + c.ToolTags = append(c.ToolTags, "goexperiment."+exp) + } + defaultToolTags = append([]string{}, c.ToolTags...) // our own private copy + // Each major Go release in the Go 1.x series adds a new // "go1.x" release tag. That is, the go1.x tag is present in // all releases >= Go 1.x. Code that requires Go 1.x or later @@ -1056,7 +1069,7 @@ func (ctxt *Context) importGo(p *Package, path, srcDir string, mode ImportMode) // we must not being doing special things like AllowBinary or IgnoreVendor, // and all the file system callbacks must be nil (we're meant to use the local file system). if mode&AllowBinary != 0 || mode&IgnoreVendor != 0 || - ctxt.JoinPath != nil || ctxt.SplitPathList != nil || ctxt.IsAbsPath != nil || ctxt.IsDir != nil || ctxt.HasSubdir != nil || ctxt.ReadDir != nil || ctxt.OpenFile != nil || !equal(ctxt.ReleaseTags, defaultReleaseTags) { + ctxt.JoinPath != nil || ctxt.SplitPathList != nil || ctxt.IsAbsPath != nil || ctxt.IsDir != nil || ctxt.HasSubdir != nil || ctxt.ReadDir != nil || ctxt.OpenFile != nil || !equal(ctxt.ToolTags, defaultToolTags) || !equal(ctxt.ReleaseTags, defaultReleaseTags) { return errNoModules } @@ -1892,6 +1905,11 @@ func (ctxt *Context) matchTag(name string, allTags map[string]bool) bool { return true } } + for _, tag := range ctxt.ToolTags { + if tag == name { + return true + } + } for _, tag := range ctxt.ReleaseTags { if tag == name { return true diff --git a/test/run.go b/test/run.go index feec2b50be..5e60de7624 100644 --- a/test/run.go +++ b/test/run.go @@ -12,6 +12,7 @@ import ( "errors" "flag" "fmt" + "go/build" "hash/fnv" "io" "io/fs" @@ -376,7 +377,6 @@ type context struct { GOARCH string cgoEnabled bool noOptEnv bool - expTags map[string]bool // Set lazily } // shouldTest looks for build tags in a source file and returns @@ -447,27 +447,12 @@ func (ctxt *context) match(name string) bool { } if strings.HasPrefix(name, "goexperiment.") { - // Query goexperiment tags from the toolchain. - if ctxt.expTags == nil { - ctxt.expTags = make(map[string]bool) - cmd := exec.Command(goTool(), "tool", "compile", "-V=goexperiment") - out, err := cmd.CombinedOutput() - if err != nil { - log.Fatalf("failed to get GOEXPERIMENT configuration:\n%s", out) - } - i := bytes.Index(out, []byte("X:")) - if i != -1 { - for _, exp := range strings.Split(string(out[i+2:]), ",") { - v := true - if strings.HasPrefix(exp, "no") { - v, exp = false, exp[2:] - } - ctxt.expTags["goexperiment."+exp] = v - } + for _, tag := range build.Default.ToolTags { + if tag == name { + return true } } - - return ctxt.expTags[name] + return false } if name == "cgo" && ctxt.cgoEnabled {