Skip to content

Commit

Permalink
internal/buildcfg: extract logic specific to cmd/go
Browse files Browse the repository at this point in the history
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 <[email protected]>
Run-TryBot: Bryan Mills <[email protected]>
Reviewed-by: Russ Cox <[email protected]>
Reviewed-by: Matthew Dempsky <[email protected]>
TryBot-Result: Gopher Robot <[email protected]>
  • Loading branch information
Bryan C. Mills committed Mar 18, 2022
1 parent d8bee94 commit 2c92b23
Show file tree
Hide file tree
Showing 12 changed files with 118 additions and 92 deletions.
2 changes: 1 addition & 1 deletion src/cmd/asm/internal/lex/input.go
Original file line number Diff line number Diff line change
Expand Up @@ -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{
Expand Down
84 changes: 52 additions & 32 deletions src/cmd/go/internal/cfg/cfg.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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 {
Expand All @@ -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.
Expand Down Expand Up @@ -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
Expand All @@ -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
Expand Down
16 changes: 13 additions & 3 deletions src/cmd/go/internal/envcmd/env.go
Original file line number Diff line number Diff line change
Expand Up @@ -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},
Expand Down Expand Up @@ -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()...)
Expand Down Expand Up @@ -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
}
}
Expand Down
4 changes: 2 additions & 2 deletions src/cmd/go/internal/load/pkg.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 != "" {
Expand Down
9 changes: 4 additions & 5 deletions src/cmd/go/internal/work/exec.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@ import (
"encoding/json"
"errors"
"fmt"
"internal/buildcfg"
exec "internal/execabs"
"internal/lazyregexp"
"io"
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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
Expand Down
3 changes: 1 addition & 2 deletions src/cmd/go/internal/work/gc.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@ import (
"bufio"
"bytes"
"fmt"
"internal/buildcfg"
"io"
"log"
"os"
Expand Down Expand Up @@ -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
}

Expand Down
3 changes: 3 additions & 0 deletions src/cmd/go/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
6 changes: 3 additions & 3 deletions src/cmd/internal/objabi/flag.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
}
Expand Down
2 changes: 1 addition & 1 deletion src/cmd/internal/objabi/util.go
Original file line number Diff line number Diff line change
Expand Up @@ -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(), ","))
}
2 changes: 1 addition & 1 deletion src/cmd/link/internal/ld/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
2 changes: 1 addition & 1 deletion src/go/build/build.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Loading

0 comments on commit 2c92b23

Please sign in to comment.