From d22f287f12f4782082cd93785ad91e78dbe0d4d6 Mon Sep 17 00:00:00 2001 From: "Bryan C. Mills" Date: Tue, 11 Apr 2023 14:27:45 +0000 Subject: [PATCH] cmd/dist: refactor generated cgo-support logic MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit During bootstrapping, cmd/dist writes a file indicating which GOOS/GOARCH combinations are valid, and which support cgo-enabled builds. That information previously went into the go/build package, but today it fits in more naturally in the internal/platform package (which already has a number of functions indicating feature support for GOOS/GOARCH combinations). Moreover, as of CL 450739 the cmd/go logic for determining whether to use cgo is somewhat more nuanced than the go/build logic: cmd/go checks for the presence of a C compiler, whereas go/build does not (mostly because it determines its configuration at package-init time, and checking $PATH for a C compiler is somewhat expensive). To simplify this situation, this change: - consolidates the “cgo supported” check in internal/platform (alongside many other platform-support checks) instead of making it a one-off in go/build, - and updates a couple of tests to use testenv.HasCGO instead of build.Default.CgoEnabled to decide whether to test a cgo-specific behavior. For #58884. For #59500. Change-Id: I0bb2502dba4545a3d98c9e915727382ce536a0f3 Reviewed-on: https://go-review.googlesource.com/c/go/+/483695 Auto-Submit: Bryan Mills Reviewed-by: Ian Lance Taylor TryBot-Result: Gopher Robot Run-TryBot: Bryan Mills --- .gitignore | 1 + src/cmd/dist/build.go | 11 ++++-- src/cmd/dist/buildgo.go | 38 +------------------ src/cmd/dist/buildruntime.go | 24 ++++++++++++ src/cmd/dist/buildtool.go | 1 + src/cmd/go/internal/cfg/cfg.go | 1 + src/cmd/go/internal/work/action.go | 3 +- src/cmd/go/internal/work/init.go | 2 +- src/cmd/go/note_test.go | 4 +- .../go/testdata/script/list_all_gobuild.txt | 1 + src/cmd/go/testdata/script/tooltags.txt | 2 + src/cmd/objdump/objdump_test.go | 3 +- src/go/build/build.go | 3 +- src/go/build/deps_test.go | 2 +- src/internal/platform/supported.go | 8 ++++ 15 files changed, 55 insertions(+), 49 deletions(-) diff --git a/.gitignore b/.gitignore index 816f59df14e..52263cdb803 100644 --- a/.gitignore +++ b/.gitignore @@ -38,6 +38,7 @@ _testmain.go /src/go/build/zcgo.go /src/go/doc/headscan /src/internal/buildcfg/zbootstrap.go +/src/internal/platform/zosarch.go /src/runtime/internal/sys/zversion.go /src/unicode/maketables /src/time/tzdata/zzipdata.go diff --git a/src/cmd/dist/build.go b/src/cmd/dist/build.go index e460b2d1cca..1d329ab9f14 100644 --- a/src/cmd/dist/build.go +++ b/src/cmd/dist/build.go @@ -626,14 +626,16 @@ var deptab = []struct { }{ {"cmd/go/internal/cfg", []string{ "zdefaultcc.go", + }}, + {"go/build", []string{ + "zcgo.go", + }}, + {"internal/platform", []string{ "zosarch.go", }}, {"runtime/internal/sys", []string{ "zversion.go", }}, - {"go/build", []string{ - "zcgo.go", - }}, {"time/tzdata", []string{ "zzipdata.go", }}, @@ -650,10 +652,10 @@ var gentab = []struct { nameprefix string gen func(string, string) }{ + {"zcgo.go", mkzcgo}, {"zdefaultcc.go", mkzdefaultcc}, {"zosarch.go", mkzosarch}, {"zversion.go", mkzversion}, - {"zcgo.go", mkzcgo}, {"zzipdata.go", mktzdata}, // not generated anymore, but delete the file if we see it @@ -1196,6 +1198,7 @@ var cleanlist = []string{ "runtime/internal/sys", "cmd/cgo", "cmd/go/internal/cfg", + "internal/platform", "go/build", } diff --git a/src/cmd/dist/buildgo.go b/src/cmd/dist/buildgo.go index d6a3b812143..2e8bb8c27d5 100644 --- a/src/cmd/dist/buildgo.go +++ b/src/cmd/dist/buildgo.go @@ -91,55 +91,19 @@ func defaultCCFunc(name string, defaultcc map[string]string) string { return buf.String() } -// mkzosarch writes zosarch.go for cmd/go. -func mkzosarch(dir, file string) { - // sort for deterministic zosarch.go file - var list []string - for plat := range cgoEnabled { - list = append(list, plat) - } - sort.Strings(list) - - var buf strings.Builder - fmt.Fprintf(&buf, "// Code generated by go tool dist; DO NOT EDIT.\n\n") - fmt.Fprintf(&buf, "package cfg\n\n") - fmt.Fprintf(&buf, "var OSArchSupportsCgo = map[string]bool{\n") - for _, plat := range list { - fmt.Fprintf(&buf, "\t%s: %v,\n", quote(plat), cgoEnabled[plat]) - } - fmt.Fprintf(&buf, "}\n") - - writefile(buf.String(), file, writeSkipSame) -} - // mkzcgo writes zcgo.go for the go/build package: // // package build -// var cgoEnabled = map[string]bool{} +// const defaultCGO_ENABLED = // // It is invoked to write go/build/zcgo.go. func mkzcgo(dir, file string) { - // sort for deterministic zcgo.go file - var list []string - for plat, hasCgo := range cgoEnabled { - if hasCgo { - list = append(list, plat) - } - } - sort.Strings(list) - var buf strings.Builder fmt.Fprintf(&buf, "// Code generated by go tool dist; DO NOT EDIT.\n") fmt.Fprintln(&buf) fmt.Fprintf(&buf, "package build\n") fmt.Fprintln(&buf) fmt.Fprintf(&buf, "const defaultCGO_ENABLED = %s\n", quote(os.Getenv("CGO_ENABLED"))) - fmt.Fprintln(&buf) - fmt.Fprintf(&buf, "var cgoEnabled = map[string]bool{\n") - for _, plat := range list { - fmt.Fprintf(&buf, "\t%s: true,\n", quote(plat)) - } - fmt.Fprintf(&buf, "}\n") writefile(buf.String(), file, writeSkipSame) } diff --git a/src/cmd/dist/buildruntime.go b/src/cmd/dist/buildruntime.go index 932c509fa4d..20f49ca744e 100644 --- a/src/cmd/dist/buildruntime.go +++ b/src/cmd/dist/buildruntime.go @@ -6,6 +6,7 @@ package main import ( "fmt" + "sort" "strings" ) @@ -82,3 +83,26 @@ func mkobjabi(file string) { writefile(buf.String(), file, writeSkipSame) } + +// mkzosarch writes zosarch.go for internal/platform. +func mkzosarch(dir, file string) { + // sort for deterministic file contents. + var list []string + for plat := range cgoEnabled { + list = append(list, plat) + } + sort.Strings(list) + + var buf strings.Builder + fmt.Fprintf(&buf, "// Code generated by go tool dist; DO NOT EDIT.\n") + fmt.Fprintln(&buf) + fmt.Fprintf(&buf, "package platform\n") + fmt.Fprintln(&buf) + fmt.Fprintf(&buf, "var osArchSupportsCgo = map[string]bool{\n") + for _, plat := range list { + fmt.Fprintf(&buf, "\t\t%s: %v,\n", quote(plat), cgoEnabled[plat]) + } + fmt.Fprintf(&buf, "}\n") + + writefile(buf.String(), file, writeSkipSame) +} diff --git a/src/cmd/dist/buildtool.go b/src/cmd/dist/buildtool.go index e06991d726e..c88e01c3de4 100644 --- a/src/cmd/dist/buildtool.go +++ b/src/cmd/dist/buildtool.go @@ -121,6 +121,7 @@ func bootstrapBuildTools() { mkbuildcfg(pathf("%s/src/internal/buildcfg/zbootstrap.go", goroot)) mkobjabi(pathf("%s/src/cmd/internal/objabi/zbootstrap.go", goroot)) + mkzosarch("", pathf("%s/src/internal/platform/zosarch.go", goroot)) // Use $GOROOT/pkg/bootstrap as the bootstrap workspace root. // We use a subdirectory of $GOROOT/pkg because that's the diff --git a/src/cmd/go/internal/cfg/cfg.go b/src/cmd/go/internal/cfg/cfg.go index ed7bb6c4bbf..edb82d01f55 100644 --- a/src/cmd/go/internal/cfg/cfg.go +++ b/src/cmd/go/internal/cfg/cfg.go @@ -141,6 +141,7 @@ func defaultContext() build.Context { // (1) environment, (2) go/env file, (3) runtime constants, // while go/build.Default.GOOS/GOARCH are derived from the preference list // (1) environment, (2) runtime constants. + // // We know ctxt.GOOS/GOARCH == runtime.GOOS/GOARCH; // no matter how that happened, go/build.Default will make the // same decision (either the environment variables are set explicitly diff --git a/src/cmd/go/internal/work/action.go b/src/cmd/go/internal/work/action.go index 123d994a9d3..de59c09d1ab 100644 --- a/src/cmd/go/internal/work/action.go +++ b/src/cmd/go/internal/work/action.go @@ -14,6 +14,7 @@ import ( "debug/elf" "encoding/json" "fmt" + "internal/platform" "os" "path/filepath" "strings" @@ -355,7 +356,7 @@ func closeBuilders() { } func CheckGOOSARCHPair(goos, goarch string) error { - if _, ok := cfg.OSArchSupportsCgo[goos+"/"+goarch]; !ok && cfg.BuildContext.Compiler == "gc" { + if !platform.BuildModeSupported(cfg.BuildContext.Compiler, "default", goos, goarch) { return fmt.Errorf("unsupported GOOS/GOARCH pair %s/%s", goos, goarch) } return nil diff --git a/src/cmd/go/internal/work/init.go b/src/cmd/go/internal/work/init.go index 88a63282855..8242e32fef4 100644 --- a/src/cmd/go/internal/work/init.go +++ b/src/cmd/go/internal/work/init.go @@ -283,7 +283,7 @@ func buildModeInit() { base.Fatalf("buildmode=%s not supported", cfg.BuildBuildmode) } - if !platform.BuildModeSupported(cfg.BuildToolchainName, cfg.BuildBuildmode, cfg.Goos, cfg.Goarch) { + if cfg.BuildBuildmode != "default" && !platform.BuildModeSupported(cfg.BuildToolchainName, cfg.BuildBuildmode, cfg.Goos, cfg.Goarch) { base.Fatalf("-buildmode=%s not supported on %s/%s\n", cfg.BuildBuildmode, cfg.Goos, cfg.Goarch) } diff --git a/src/cmd/go/note_test.go b/src/cmd/go/note_test.go index 659366dcf97..ba7ec2a47bc 100644 --- a/src/cmd/go/note_test.go +++ b/src/cmd/go/note_test.go @@ -5,7 +5,7 @@ package main_test import ( - "go/build" + "internal/testenv" "runtime" "testing" @@ -32,7 +32,7 @@ func TestNoteReading(t *testing.T) { } switch { - case !build.Default.CgoEnabled: + case !testenv.HasCGO(): t.Skipf("skipping - no cgo, so assuming external linking not available") case runtime.GOOS == "plan9": t.Skipf("skipping - external linking not supported") diff --git a/src/cmd/go/testdata/script/list_all_gobuild.txt b/src/cmd/go/testdata/script/list_all_gobuild.txt index e0a47398bbd..92ad7d88566 100644 --- a/src/cmd/go/testdata/script/list_all_gobuild.txt +++ b/src/cmd/go/testdata/script/list_all_gobuild.txt @@ -1,5 +1,6 @@ # go list all should work with GOOS=linux because all packages build on Linux env GOOS=linux +env GOARCH=amd64 go list all # go list all should work with GOOS=darwin, but it used to fail because diff --git a/src/cmd/go/testdata/script/tooltags.txt b/src/cmd/go/testdata/script/tooltags.txt index 3076185bda7..27068eebaeb 100644 --- a/src/cmd/go/testdata/script/tooltags.txt +++ b/src/cmd/go/testdata/script/tooltags.txt @@ -1,3 +1,5 @@ +env GOOS=linux + env GOARCH=amd64 env GOAMD64=v3 go list -f '{{context.ToolTags}}' diff --git a/src/cmd/objdump/objdump_test.go b/src/cmd/objdump/objdump_test.go index 3abfb1461c0..6e781c924d2 100644 --- a/src/cmd/objdump/objdump_test.go +++ b/src/cmd/objdump/objdump_test.go @@ -8,7 +8,6 @@ import ( "cmd/internal/notsha256" "flag" "fmt" - "go/build" "internal/platform" "internal/testenv" "os" @@ -253,7 +252,7 @@ func testDisasm(t *testing.T, srcfname string, printCode bool, printGnuAsm bool, func testGoAndCgoDisasm(t *testing.T, printCode bool, printGnuAsm bool) { t.Parallel() testDisasm(t, "fmthello.go", printCode, printGnuAsm) - if build.Default.CgoEnabled { + if testenv.HasCGO() { testDisasm(t, "fmthellocgo.go", printCode, printGnuAsm) } } diff --git a/src/go/build/build.go b/src/go/build/build.go index d20964e60b2..dd6cdc903a2 100644 --- a/src/go/build/build.go +++ b/src/go/build/build.go @@ -16,6 +16,7 @@ import ( "internal/godebug" "internal/goroot" "internal/goversion" + "internal/platform" "io" "io/fs" "os" @@ -345,7 +346,7 @@ func defaultContext() Context { default: // cgo must be explicitly enabled for cross compilation builds if runtime.GOARCH == c.GOARCH && runtime.GOOS == c.GOOS { - c.CgoEnabled = cgoEnabled[c.GOOS+"/"+c.GOARCH] + c.CgoEnabled = platform.CgoSupported(c.GOOS, c.GOARCH) break } c.CgoEnabled = false diff --git a/src/go/build/deps_test.go b/src/go/build/deps_test.go index 4415b92a295..c0f3cfd780f 100644 --- a/src/go/build/deps_test.go +++ b/src/go/build/deps_test.go @@ -292,7 +292,7 @@ var depsRules = ` FMT, internal/goexperiment < internal/buildcfg; - go/build/constraint, go/doc, go/parser, internal/buildcfg, internal/goroot, internal/goversion + go/build/constraint, go/doc, go/parser, internal/buildcfg, internal/goroot, internal/goversion, internal/platform < go/build; # databases diff --git a/src/internal/platform/supported.go b/src/internal/platform/supported.go index 01524fbcd77..57a86b054d3 100644 --- a/src/internal/platform/supported.go +++ b/src/internal/platform/supported.go @@ -126,6 +126,9 @@ func BuildModeSupported(compiler, buildmode, goos, goarch string) bool { } platform := goos + "/" + goarch + if _, ok := osArchSupportsCgo[platform]; !ok { + return false // platform unrecognized + } switch buildmode { case "archive": @@ -237,3 +240,8 @@ func DefaultPIE(goos, goarch string, isRace bool) bool { } return false } + +// CgoSupported reports whether goos/goarch supports cgo.\n") +func CgoSupported(goos, goarch string) bool { + return osArchSupportsCgo[goos+"/"+goarch] +}