]> Cypherpunks repositories - gostls13.git/commitdiff
internal/buildcfg: extract logic specific to cmd/go
authorBryan C. Mills <bcmills@google.com>
Wed, 16 Mar 2022 20:25:47 +0000 (16:25 -0400)
committerBryan Mills <bcmills@google.com>
Fri, 18 Mar 2022 15:35:06 +0000 (15:35 +0000)
cmd/go/internal/cfg duplicates many of the fields of
internal/buildcfg, but initializes them from a Go environment file in
addition to the usual process environment.

internal/buildcfg doesn't (and shouldn't) know or care about that
environment file, but prior to this CL it exposed hooks for
cmd/go/internal/cfg to write data back to internal/buildcfg to
incorporate information from the file. It also produced quirky
GOEXPERIMENT strings when a non-trivial default was overridden,
seemingly so that 'go env' would produce those same quirky strings in
edge-cases where they are needed.

This change reverses that information flow: internal/buildcfg now
exports a structured type with methods — instead of top-level
functions communicating through global state — so that cmd/go can
utilize its marshaling and unmarshaling functionality without also
needing to write results back into buildcfg package state.

The quirks specific to 'go env' have been eliminated by distinguishing
between the raw GOEXPERIMENT value set by the user (which is what we
should report from 'go env') and the cleaned, canonical equivalent
(which is what we should use in the build cache key).

For #51461.

Change-Id: I4ef5b7c58b1fb3468497649a6d2fb6c19aa06c70
Reviewed-on: https://go-review.googlesource.com/c/go/+/393574
Trust: Bryan Mills <bcmills@google.com>
Run-TryBot: Bryan Mills <bcmills@google.com>
Reviewed-by: Russ Cox <rsc@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>

12 files changed:
src/cmd/asm/internal/lex/input.go
src/cmd/go/internal/cfg/cfg.go
src/cmd/go/internal/envcmd/env.go
src/cmd/go/internal/load/pkg.go
src/cmd/go/internal/work/exec.go
src/cmd/go/internal/work/gc.go
src/cmd/go/main.go
src/cmd/internal/objabi/flag.go
src/cmd/internal/objabi/util.go
src/cmd/link/internal/ld/main.go
src/go/build/build.go
src/internal/buildcfg/exp.go

index e373ae817e0d83f70bdc3ae5fe7b58f89c179f26..276b4b0dcd6cd1accf0b1f2e626fe0cefc0b9b50 100644 (file)
@@ -50,7 +50,7 @@ func predefine(defines flags.MultiFlag) map[string]*Macro {
        // Set macros for GOEXPERIMENTs so we can easily switch
        // runtime assembly code based on them.
        if *flags.CompilingRuntime {
-               for _, exp := range buildcfg.EnabledExperiments() {
+               for _, exp := range buildcfg.Experiment.Enabled() {
                        // Define macro.
                        name := "GOEXPERIMENT_" + exp
                        macros[name] = &Macro{
index deab3dddd0dbeaa94e136422d750a8a42bd2c03e..77c0e229e5e4e2da1bcac7a0334e64e63632e867 100644 (file)
@@ -22,6 +22,26 @@ import (
        "cmd/go/internal/fsys"
 )
 
+// Global build parameters (used during package load)
+var (
+       Goos   = envOr("GOOS", build.Default.GOOS)
+       Goarch = envOr("GOARCH", build.Default.GOARCH)
+
+       ExeSuffix = exeSuffix()
+
+       // ModulesEnabled specifies whether the go command is running
+       // in module-aware mode (as opposed to GOPATH mode).
+       // It is equal to modload.Enabled, but not all packages can import modload.
+       ModulesEnabled bool
+)
+
+func exeSuffix() string {
+       if Goos == "windows" {
+               return ".exe"
+       }
+       return ""
+}
+
 // These are general "build flags" used by build and other commands.
 var (
        BuildA                 bool   // -a flag
@@ -60,8 +80,6 @@ var (
        // GoPathError is set when GOPATH is not set. it contains an
        // explanation why GOPATH is unset.
        GoPathError string
-
-       GOEXPERIMENT = envOr("GOEXPERIMENT", buildcfg.DefaultGOEXPERIMENT)
 )
 
 func defaultContext() build.Context {
@@ -79,20 +97,15 @@ func defaultContext() build.Context {
                build.ToolDir = filepath.Join(ctxt.GOROOT, "pkg/tool/"+runtime.GOOS+"_"+runtime.GOARCH)
        }
 
-       ctxt.GOPATH = envOr("GOPATH", gopath(ctxt))
-
        // Override defaults computed in go/build with defaults
        // from go environment configuration file, if known.
-       ctxt.GOOS = envOr("GOOS", ctxt.GOOS)
-       ctxt.GOARCH = envOr("GOARCH", ctxt.GOARCH)
+       ctxt.GOPATH = envOr("GOPATH", gopath(ctxt))
+       ctxt.GOOS = Goos
+       ctxt.GOARCH = Goarch
 
-       // The experiments flags are based on GOARCH, so they may
-       // need to change.  TODO: This should be cleaned up.
-       buildcfg.UpdateExperiments(ctxt.GOOS, ctxt.GOARCH, GOEXPERIMENT)
+       // ToolTags are based on GOEXPERIMENT, which we will parse and
+       // initialize later.
        ctxt.ToolTags = nil
-       for _, exp := range buildcfg.EnabledExperiments() {
-               ctxt.ToolTags = append(ctxt.ToolTags, "goexperiment."+exp)
-       }
 
        // The go/build rule for whether cgo is enabled is:
        //      1. If $CGO_ENABLED is set, respect it.
@@ -137,6 +150,33 @@ func init() {
        BuildToolchainLinker = func() string { return "missing-linker" }
 }
 
+// Experiment configuration.
+var (
+       // RawGOEXPERIMENT is the GOEXPERIMENT value set by the user.
+       RawGOEXPERIMENT = envOr("GOEXPERIMENT", buildcfg.DefaultGOEXPERIMENT)
+       // CleanGOEXPERIMENT is the minimal GOEXPERIMENT value needed to reproduce the
+       // experiments enabled by RawGOEXPERIMENT.
+       CleanGOEXPERIMENT = RawGOEXPERIMENT
+
+       Experiment    *buildcfg.ExperimentFlags
+       ExperimentErr error
+)
+
+func init() {
+       Experiment, ExperimentErr = buildcfg.ParseGOEXPERIMENT(Goos, Goarch, RawGOEXPERIMENT)
+       if ExperimentErr != nil {
+               return
+       }
+
+       // GOEXPERIMENT is valid, so convert it to canonical form.
+       CleanGOEXPERIMENT = Experiment.String()
+
+       // Add build tags based on the experiments in effect.
+       for _, exp := range Experiment.Enabled() {
+               BuildContext.ToolTags = append(BuildContext.ToolTags, "goexperiment."+exp)
+       }
+}
+
 // An EnvVar is an environment variable Name=Value.
 type EnvVar struct {
        Name  string
@@ -151,26 +191,6 @@ var OrigEnv []string
 // not CmdEnv.
 var CmdEnv []EnvVar
 
-// Global build parameters (used during package load)
-var (
-       Goarch = BuildContext.GOARCH
-       Goos   = BuildContext.GOOS
-
-       ExeSuffix = exeSuffix()
-
-       // ModulesEnabled specifies whether the go command is running
-       // in module-aware mode (as opposed to GOPATH mode).
-       // It is equal to modload.Enabled, but not all packages can import modload.
-       ModulesEnabled bool
-)
-
-func exeSuffix() string {
-       if Goos == "windows" {
-               return ".exe"
-       }
-       return ""
-}
-
 var envCache struct {
        once sync.Once
        m    map[string]string
index c1adf8cef4ca1b386042ff17c0da3d9245c9cfb8..fcabc8d1c71a3fc4f5f5f164f22724846b737840 100644 (file)
@@ -74,7 +74,14 @@ func MkEnv() []cfg.EnvVar {
                {Name: "GOCACHE", Value: cache.DefaultDir()},
                {Name: "GOENV", Value: envFile},
                {Name: "GOEXE", Value: cfg.ExeSuffix},
-               {Name: "GOEXPERIMENT", Value: buildcfg.GOEXPERIMENT()},
+
+               // List the raw value of GOEXPERIMENT, not the cleaned one.
+               // The set of default experiments may change from one release
+               // to the next, so a GOEXPERIMENT setting that is redundant
+               // with the current toolchain might actually be relevant with
+               // a different version (for example, when bisecting a regression).
+               {Name: "GOEXPERIMENT", Value: cfg.RawGOEXPERIMENT},
+
                {Name: "GOFLAGS", Value: cfg.Getenv("GOFLAGS")},
                {Name: "GOHOSTARCH", Value: runtime.GOARCH},
                {Name: "GOHOSTOS", Value: runtime.GOOS},
@@ -222,6 +229,9 @@ func runEnv(ctx context.Context, cmd *base.Command, args []string) {
        }
 
        buildcfg.Check()
+       if cfg.ExperimentErr != nil {
+               base.Fatalf("go: %v", cfg.ExperimentErr)
+       }
 
        env := cfg.CmdEnv
        env = append(env, ExtraEnvVars()...)
@@ -374,9 +384,9 @@ func checkBuildConfig(add map[string]string, del map[string]bool) error {
                }
        }
 
-       goexperiment, okGOEXPERIMENT := get("GOEXPERIMENT", buildcfg.GOEXPERIMENT(), "")
+       goexperiment, okGOEXPERIMENT := get("GOEXPERIMENT", cfg.RawGOEXPERIMENT, buildcfg.DefaultGOEXPERIMENT)
        if okGOEXPERIMENT {
-               if _, _, err := buildcfg.ParseGOEXPERIMENT(goos, goarch, goexperiment); err != nil {
+               if _, err := buildcfg.ParseGOEXPERIMENT(goos, goarch, goexperiment); err != nil {
                        return err
                }
        }
index fdc00f95dca4a6ac229573f379f52a6eba763efd..8c169d1643a0a4ede814f7d2ed02841650391f75 100644 (file)
@@ -2334,8 +2334,8 @@ func (p *Package) setBuildInfo() {
                        }
                }
                appendSetting("GOARCH", cfg.BuildContext.GOARCH)
-               if cfg.GOEXPERIMENT != "" {
-                       appendSetting("GOEXPERIMENT", cfg.GOEXPERIMENT)
+               if cfg.RawGOEXPERIMENT != "" {
+                       appendSetting("GOEXPERIMENT", cfg.RawGOEXPERIMENT)
                }
                appendSetting("GOOS", cfg.BuildContext.GOOS)
                if key, val := cfg.GetArchEnv(); key != "" && val != "" {
index 7d3a16c5f51c218cbefe41f6921a293f959fc6e1..6d6837aa8a2b7da2d33066ed1b49c6d33fd051af 100644 (file)
@@ -12,7 +12,6 @@ import (
        "encoding/json"
        "errors"
        "fmt"
-       "internal/buildcfg"
        exec "internal/execabs"
        "internal/lazyregexp"
        "io"
@@ -320,8 +319,8 @@ func (b *Builder) buildActionID(a *Action) cache.ActionID {
                key, val := cfg.GetArchEnv()
                fmt.Fprintf(h, "%s=%s\n", key, val)
 
-               if goexperiment := buildcfg.GOEXPERIMENT(); goexperiment != "" {
-                       fmt.Fprintf(h, "GOEXPERIMENT=%q\n", goexperiment)
+               if cfg.CleanGOEXPERIMENT != "" {
+                       fmt.Fprintf(h, "GOEXPERIMENT=%q\n", cfg.CleanGOEXPERIMENT)
                }
 
                // TODO(rsc): Convince compiler team not to add more magic environment variables,
@@ -1301,8 +1300,8 @@ func (b *Builder) printLinkerConfig(h io.Writer, p *load.Package) {
                key, val := cfg.GetArchEnv()
                fmt.Fprintf(h, "%s=%s\n", key, val)
 
-               if goexperiment := buildcfg.GOEXPERIMENT(); goexperiment != "" {
-                       fmt.Fprintf(h, "GOEXPERIMENT=%q\n", goexperiment)
+               if cfg.CleanGOEXPERIMENT != "" {
+                       fmt.Fprintf(h, "GOEXPERIMENT=%q\n", cfg.CleanGOEXPERIMENT)
                }
 
                // The linker writes source file paths that say GOROOT_FINAL, but
index e1e2b11dd73409eed40759f55125cbf477fdcdcc..a6174b2ed2891ccce318f31b0ea5dcf6ab502296 100644 (file)
@@ -8,7 +8,6 @@ import (
        "bufio"
        "bytes"
        "fmt"
-       "internal/buildcfg"
        "io"
        "log"
        "os"
@@ -245,7 +244,7 @@ CheckFlags:
        }
 
        // TODO: Test and delete these conditions.
-       if buildcfg.Experiment.FieldTrack || buildcfg.Experiment.PreemptibleLoops {
+       if cfg.ExperimentErr != nil || cfg.Experiment.FieldTrack || cfg.Experiment.PreemptibleLoops {
                canDashC = false
        }
 
index c0a1d3ccfc496cd04d7d3edc3d864455dddbcee9..ed46ed822a601e0a335aea6c9fcff801142252fc 100644 (file)
@@ -190,6 +190,9 @@ func invoke(cmd *base.Command, args []string) {
        // 'go env' handles checking the build config
        if cmd != envcmd.CmdEnv {
                buildcfg.Check()
+               if cfg.ExperimentErr != nil {
+                       base.Fatalf("go: %v", cfg.ExperimentErr)
+               }
        }
 
        // Set environment (GOOS, GOARCH, etc) explicitly.
index f75c054fcb6110c332dfd12d5ec9fdd66d47b5fe..acb2dd59eaf6849b5760222656aeed476f806408 100644 (file)
@@ -99,11 +99,11 @@ func (versionFlag) Set(s string) error {
        if s == "goexperiment" {
                // test/run.go uses this to discover the full set of
                // experiment tags. Report everything.
-               p = " X:" + strings.Join(buildcfg.AllExperiments(), ",")
+               p = " X:" + strings.Join(buildcfg.Experiment.All(), ",")
        } else {
-               // If the enabled experiments differ from the defaults,
+               // If the enabled experiments differ from the baseline,
                // include that difference.
-               if goexperiment := buildcfg.GOEXPERIMENT(); goexperiment != "" {
+               if goexperiment := buildcfg.Experiment.String(); goexperiment != "" {
                        p = " X:" + goexperiment
                }
        }
index 6bfa25a5caec05da06a1bd5cb3c9b725f642b85c..c2f1b204b980f21595bd594ba9aed58ec6fbef8c 100644 (file)
@@ -22,5 +22,5 @@ const (
 // or link object files that are incompatible with each other. This
 // string always starts with "go object ".
 func HeaderString() string {
-       return fmt.Sprintf("go object %s %s %s X:%s\n", buildcfg.GOOS, buildcfg.GOARCH, buildcfg.Version, strings.Join(buildcfg.EnabledExperiments(), ","))
+       return fmt.Sprintf("go object %s %s %s X:%s\n", buildcfg.GOOS, buildcfg.GOARCH, buildcfg.Version, strings.Join(buildcfg.Experiment.Enabled(), ","))
 }
index 26f9db8ec410bf3dbc5a2265c51cc95d6700863d..d13c3ff8b61cc2b9adce7fe5e5b1009c3f3edbea 100644 (file)
@@ -124,7 +124,7 @@ func Main(arch *sys.Arch, theArch Arch) {
        addstrdata1(ctxt, "internal/buildcfg.defaultGOROOT="+final)
 
        buildVersion := buildcfg.Version
-       if goexperiment := buildcfg.GOEXPERIMENT(); goexperiment != "" {
+       if goexperiment := buildcfg.Experiment.String(); goexperiment != "" {
                buildVersion += " X:" + goexperiment
        }
        addstrdata1(ctxt, "runtime.buildVersion="+buildVersion)
index c1d044e55a8f5188c53581321d12fae90f875725..b0842b3a1a689e72a1cdabac48e1643570e7ae6b 100644 (file)
@@ -311,7 +311,7 @@ func defaultContext() Context {
        // 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() {
+       for _, exp := range buildcfg.Experiment.Enabled() {
                c.ToolTags = append(c.ToolTags, "goexperiment."+exp)
        }
        defaultToolTags = append([]string{}, c.ToolTags...) // our own private copy
index 230ec0b231a8bfaa955d7b98cc80ba2192ebb882..a56b36efdf52e1075d64064f0256a11068f4e321 100644 (file)
@@ -12,6 +12,13 @@ import (
        "internal/goexperiment"
 )
 
+// ExperimentFlags represents a set of GOEXPERIMENT flags relative to a baseline
+// (platform-default) experiment configuration.
+type ExperimentFlags struct {
+       goexperiment.Flags
+       baseline goexperiment.Flags
+}
+
 // Experiment contains the toolchain experiments enabled for the
 // current build.
 //
@@ -21,14 +28,17 @@ import (
 // experimentBaseline specifies the experiment flags that are enabled by
 // default in the current toolchain. This is, in effect, the "control"
 // configuration and any variation from this is an experiment.
-var Experiment, experimentBaseline = func() (goexperiment.Flags, goexperiment.Flags) {
-       flags, baseline, err := ParseGOEXPERIMENT(GOOS, GOARCH, envOr("GOEXPERIMENT", defaultGOEXPERIMENT))
+var Experiment ExperimentFlags = func() ExperimentFlags {
+       flags, err := ParseGOEXPERIMENT(GOOS, GOARCH, envOr("GOEXPERIMENT", defaultGOEXPERIMENT))
        if err != nil {
                Error = err
+               return ExperimentFlags{}
        }
-       return flags, baseline
+       return *flags
 }()
 
+// DefaultGOEXPERIMENT is the embedded default GOEXPERIMENT string.
+// It is not guaranteed to be canonical.
 const DefaultGOEXPERIMENT = defaultGOEXPERIMENT
 
 // FramePointerEnabled enables the use of platform conventions for
@@ -45,21 +55,24 @@ var FramePointerEnabled = GOARCH == "amd64" || GOARCH == "arm64"
 // flag sets.
 //
 // TODO(mdempsky): Move to internal/goexperiment.
-func ParseGOEXPERIMENT(goos, goarch, goexp string) (flags, baseline goexperiment.Flags, err error) {
+func ParseGOEXPERIMENT(goos, goarch, goexp string) (*ExperimentFlags, error) {
        regabiSupported := false
        switch goarch {
        case "amd64", "arm64", "ppc64le", "ppc64":
                regabiSupported = true
        }
 
-       baseline = goexperiment.Flags{
+       baseline := goexperiment.Flags{
                RegabiWrappers: regabiSupported,
                RegabiArgs:     regabiSupported,
                PacerRedesign:  true,
        }
 
        // Start with the statically enabled set of experiments.
-       flags = baseline
+       flags := &ExperimentFlags{
+               Flags:    baseline,
+               baseline: baseline,
+       }
 
        // Pick up any changes to the baseline configuration from the
        // GOEXPERIMENT environment. This can be set at make.bash time
@@ -67,7 +80,7 @@ func ParseGOEXPERIMENT(goos, goarch, goexp string) (flags, baseline goexperiment
        if goexp != "" {
                // Create a map of known experiment names.
                names := make(map[string]func(bool))
-               rv := reflect.ValueOf(&flags).Elem()
+               rv := reflect.ValueOf(&flags.Flags).Elem()
                rt := rv.Type()
                for i := 0; i < rt.NumField(); i++ {
                        field := rv.Field(i)
@@ -92,7 +105,7 @@ func ParseGOEXPERIMENT(goos, goarch, goexp string) (flags, baseline goexperiment
                                // GOEXPERIMENT=none disables all experiment flags.
                                // This is used by cmd/dist, which doesn't know how
                                // to build with any experiment flags.
-                               flags = goexperiment.Flags{}
+                               flags.Flags = goexperiment.Flags{}
                                continue
                        }
                        val := true
@@ -101,8 +114,7 @@ func ParseGOEXPERIMENT(goos, goarch, goexp string) (flags, baseline goexperiment
                        }
                        set, ok := names[f]
                        if !ok {
-                               err = fmt.Errorf("unknown GOEXPERIMENT %s", f)
-                               return
+                               return nil, fmt.Errorf("unknown GOEXPERIMENT %s", f)
                        }
                        set(val)
                }
@@ -119,9 +131,15 @@ func ParseGOEXPERIMENT(goos, goarch, goexp string) (flags, baseline goexperiment
        }
        // Check regabi dependencies.
        if flags.RegabiArgs && !flags.RegabiWrappers {
-               err = fmt.Errorf("GOEXPERIMENT regabiargs requires regabiwrappers")
+               return nil, fmt.Errorf("GOEXPERIMENT regabiargs requires regabiwrappers")
        }
-       return
+       return flags, nil
+}
+
+// String returns the canonical GOEXPERIMENT string to enable this experiment
+// configuration. (Experiments in the same state as in the baseline are elided.)
+func (exp *ExperimentFlags) String() string {
+       return strings.Join(expList(&exp.Flags, &exp.baseline, false), ",")
 }
 
 // expList returns the list of lower-cased experiment names for
@@ -154,37 +172,14 @@ func expList(exp, base *goexperiment.Flags, all bool) []string {
        return list
 }
 
-// GOEXPERIMENT is a comma-separated list of enabled or disabled
-// experiments that differ from the baseline experiment configuration.
-// GOEXPERIMENT is exactly what a user would set on the command line
-// to get the set of enabled experiments.
-func GOEXPERIMENT() string {
-       goexp := strings.Join(expList(&Experiment, &experimentBaseline, false), ",")
-       if goexp == "" && DefaultGOEXPERIMENT != "" {
-               goexp = "," // non-empty to override DefaultGOEXPERIMENT
-       }
-       return goexp
-}
-
-// EnabledExperiments returns a list of enabled experiments, as
+// Enabled returns a list of enabled experiments, as
 // lower-cased experiment names.
-func EnabledExperiments() []string {
-       return expList(&Experiment, nil, false)
+func (exp *ExperimentFlags) Enabled() []string {
+       return expList(&exp.Flags, nil, false)
 }
 
-// AllExperiments returns a list of all experiment settings.
+// All returns a list of all experiment settings.
 // Disabled experiments appear in the list prefixed by "no".
-func AllExperiments() []string {
-       return expList(&Experiment, nil, true)
-}
-
-// UpdateExperiments updates the Experiment global based on a new GOARCH value.
-// This is only required for cmd/go, which can change GOARCH after
-// program startup due to use of "go env -w".
-func UpdateExperiments(goos, goarch, goexperiment string) {
-       var err error
-       Experiment, experimentBaseline, err = ParseGOEXPERIMENT(goos, goarch, goexperiment)
-       if err != nil {
-               Error = err
-       }
+func (exp *ExperimentFlags) All() []string {
+       return expList(&exp.Flags, nil, true)
 }