Skip to content

Commit

Permalink
cmd/dist: leave cgo enabled if external linking is required
Browse files Browse the repository at this point in the history
Certain ios and android configurations do not yet support internal
linking.

On ios, attempting to build without cgo causes tests to fail on
essentially every run (golang#57961).

On android, it produces a lot of warning spam from the linker,
obscuring real problems.

Since external linking makes the result of `go install` depend on the
installed C toolchain either way, the reproducibility benefit of
disabling cgo seems minimal on these platforms anyway.

Fixes golang#57961.
For golang#24904.
Updates golang#57007.

Change-Id: Ied2454804e958dd670467db3d5e9ab50a40bb899
Reviewed-on: https://go-review.googlesource.com/c/go/+/463739
Reviewed-by: Russ Cox <[email protected]>
Run-TryBot: Bryan Mills <[email protected]>
Auto-Submit: Bryan Mills <[email protected]>
Reviewed-by: Cherry Mui <[email protected]>
TryBot-Bypass: Bryan Mills <[email protected]>
  • Loading branch information
Bryan C. Mills authored and johanbrandhorst committed Feb 12, 2023
1 parent 1b4b504 commit 2719f41
Show file tree
Hide file tree
Showing 3 changed files with 87 additions and 33 deletions.
88 changes: 59 additions & 29 deletions src/cmd/dist/build.go
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,7 @@ var (

rebuildall bool
noOpt bool
isRelease bool

vflag int // verbosity
)
Expand Down Expand Up @@ -257,6 +258,9 @@ func xinit() {
xatexit(rmworkdir)

tooldir = pathf("%s/pkg/tool/%s_%s", goroot, gohostos, gohostarch)

goversion := findgoversion()
isRelease = strings.HasPrefix(goversion, "release.") || strings.HasPrefix(goversion, "go")
}

// compilerEnv returns a map from "goos/goarch" to the
Expand Down Expand Up @@ -302,7 +306,7 @@ func compilerEnv(envName, def string) map[string]string {

// clangos lists the operating systems where we prefer clang to gcc.
var clangos = []string{
"darwin", // macOS 10.9 and later require clang
"darwin", "ios", // macOS 10.9 and later require clang
"freebsd", // FreeBSD 10 and later do not ship gcc
"openbsd", // OpenBSD ships with GCC 4.2, which is now quite old.
}
Expand Down Expand Up @@ -519,8 +523,6 @@ func setup() {
}

// Special release-specific setup.
goversion := findgoversion()
isRelease := strings.HasPrefix(goversion, "release.") || (strings.HasPrefix(goversion, "go") && !strings.Contains(goversion, "beta"))
if isRelease {
// Make sure release-excluded things are excluded.
for _, dir := range unreleased {
Expand All @@ -529,20 +531,29 @@ func setup() {
}
}
}
if isRelease || os.Getenv("GO_BUILDER_NAME") != "" {
// Add -trimpath for reproducible builds of releases.
// Include builders so that -trimpath is well-tested ahead of releases.
// Do not include local development, so that people working in the
// main branch for day-to-day work on the Go toolchain itself can
// still have full paths for stack traces for compiler crashes and the like.
// toolenv = append(toolenv, "GOFLAGS=-trimpath")
}
}

/*
* Tool building
*/

// mustLinkExternal is a copy of internal/platform.MustLinkExternal,
// duplicated here to avoid version skew in the MustLinkExternal function
// during bootstrapping.
func mustLinkExternal(goos, goarch string) bool {
switch goos {
case "android":
if goarch != "arm64" {
return true
}
case "ios":
if goarch == "arm64" {
return true
}
}
return false
}

// deptab lists changes to the default dependencies for a given prefix.
// deps ending in /* read the whole directory; deps beginning with -
// exclude files with that prefix.
Expand Down Expand Up @@ -1266,14 +1277,33 @@ func timelog(op, name string) {
fmt.Fprintf(timeLogFile, "%s %+.1fs %s %s\n", t.Format(time.UnixDate), t.Sub(timeLogStart).Seconds(), op, name)
}

// toolenv is the environment to use when building cmd.
// We disable cgo to get static binaries for cmd/go and cmd/pprof,
// so that they work on systems without the same dynamic libraries
// as the original build system.
// In release branches, we add -trimpath for reproducible builds.
// In the main branch we leave it off, so that compiler crashes and
// the like have full path names for easier navigation to source files.
var toolenv = []string{"CGO_ENABLED=0"}
// toolenv returns the environment to use when building commands in cmd.
//
// This is a function instead of a variable because the exact toolenv depends
// on the GOOS and GOARCH, and (at least for now) those are modified in place
// to switch between the host and target configurations when cross-compiling.
func toolenv() []string {
var env []string
if !mustLinkExternal(goos, goarch) {
// Unless the platform requires external linking,
// we disable cgo to get static binaries for cmd/go and cmd/pprof,
// so that they work on systems without the same dynamic libraries
// as the original build system.
env = append(env, "CGO_ENABLED=0")
}
if isRelease || os.Getenv("GO_BUILDER_NAME") != "" {
// Add -trimpath for reproducible builds of releases.
// Include builders so that -trimpath is well-tested ahead of releases.
// Do not include local development, so that people working in the
// main branch for day-to-day work on the Go toolchain itself can
// still have full paths for stack traces for compiler crashes and the like.
//
// TODO(bcmills): This was added but commented out in CL 454836.
// Uncomment or delete it.
// env = append(env, "GOFLAGS=-trimpath")
}
return env
}

var toolchain = []string{"cmd/asm", "cmd/cgo", "cmd/compile", "cmd/link"}

Expand Down Expand Up @@ -1414,7 +1444,7 @@ func cmdbootstrap() {
os.Setenv("CC", compilerEnvLookup("CC", defaultcc, goos, goarch))
// Now that cmd/go is in charge of the build process, enable GOEXPERIMENT.
os.Setenv("GOEXPERIMENT", goexperiment)
goInstall(toolenv, goBootstrap, toolchain...)
goInstall(toolenv(), goBootstrap, toolchain...)
if debug {
run("", ShowOutput|CheckExit, pathf("%s/compile", tooldir), "-V=full")
copyfile(pathf("%s/compile2", tooldir), pathf("%s/compile", tooldir), writeExec)
Expand All @@ -1441,7 +1471,7 @@ func cmdbootstrap() {
xprintf("\n")
}
xprintf("Building Go toolchain3 using go_bootstrap and Go toolchain2.\n")
goInstall(toolenv, goBootstrap, append([]string{"-a"}, toolchain...)...)
goInstall(toolenv(), goBootstrap, append([]string{"-a"}, toolchain...)...)
if debug {
run("", ShowOutput|CheckExit, pathf("%s/compile", tooldir), "-V=full")
copyfile(pathf("%s/compile3", tooldir), pathf("%s/compile", tooldir), writeExec)
Expand Down Expand Up @@ -1470,9 +1500,9 @@ func cmdbootstrap() {
xprintf("\n")
}
xprintf("Building commands for host, %s/%s.\n", goos, goarch)
goInstall(toolenv, goBootstrap, "cmd")
checkNotStale(toolenv, goBootstrap, "cmd")
checkNotStale(toolenv, gorootBinGo, "cmd")
goInstall(toolenv(), goBootstrap, "cmd")
checkNotStale(toolenv(), goBootstrap, "cmd")
checkNotStale(toolenv(), gorootBinGo, "cmd")

timelog("build", "target toolchain")
if vflag > 0 {
Expand All @@ -1486,15 +1516,15 @@ func cmdbootstrap() {
xprintf("Building packages and commands for target, %s/%s.\n", goos, goarch)
}
goInstall(nil, goBootstrap, "std")
goInstall(toolenv, goBootstrap, "cmd")
checkNotStale(toolenv, goBootstrap, append(toolchain, "runtime/internal/sys")...)
goInstall(toolenv(), goBootstrap, "cmd")
checkNotStale(toolenv(), goBootstrap, append(toolchain, "runtime/internal/sys")...)
checkNotStale(nil, goBootstrap, "std")
checkNotStale(toolenv, goBootstrap, "cmd")
checkNotStale(toolenv(), goBootstrap, "cmd")
checkNotStale(nil, gorootBinGo, "std")
checkNotStale(toolenv, gorootBinGo, "cmd")
checkNotStale(toolenv(), gorootBinGo, "cmd")
if debug {
run("", ShowOutput|CheckExit, pathf("%s/compile", tooldir), "-V=full")
checkNotStale(toolenv, goBootstrap, append(toolchain, "runtime/internal/sys")...)
checkNotStale(toolenv(), goBootstrap, append(toolchain, "runtime/internal/sys")...)
copyfile(pathf("%s/compile4", tooldir), pathf("%s/compile", tooldir), writeExec)
}

Expand Down
24 changes: 24 additions & 0 deletions src/cmd/dist/build_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
// Copyright 2023 The Go Authors. All rights reserved.
// Use of this source code is governed by a BSD-style
// license that can be found in the LICENSE file.

package main

import (
"internal/platform"
"testing"
)

// TestMustLinkExternal verifies that the mustLinkExternal helper
// function matches internal/platform.MustLinkExternal.
func TestMustLinkExternal(t *testing.T) {
for _, goos := range okgoos {
for _, goarch := range okgoarch {
got := mustLinkExternal(goos, goarch)
want := platform.MustLinkExternal(goos, goarch)
if got != want {
t.Errorf("mustLinkExternal(%q, %q) = %v; want %v", goos, goarch, got, want)
}
}
}
}
8 changes: 4 additions & 4 deletions src/cmd/dist/test.go
Original file line number Diff line number Diff line change
Expand Up @@ -149,7 +149,7 @@ func (t *tester) run() {
if t.rebuild {
t.out("Building packages and commands.")
// Force rebuild the whole toolchain.
goInstall(toolenv, gorootBinGo, append([]string{"-a"}, toolchain...)...)
goInstall(toolenv(), gorootBinGo, append([]string{"-a"}, toolchain...)...)
}

if !t.listMode {
Expand All @@ -166,9 +166,9 @@ func (t *tester) run() {
// and virtualization we usually start with a clean GOCACHE, so we would
// end up rebuilding large parts of the standard library that aren't
// otherwise relevant to the actual set of packages under test.
goInstall(toolenv, gorootBinGo, toolchain...)
goInstall(toolenv, gorootBinGo, toolchain...)
goInstall(toolenv, gorootBinGo, "cmd")
goInstall(toolenv(), gorootBinGo, toolchain...)
goInstall(toolenv(), gorootBinGo, toolchain...)
goInstall(toolenv(), gorootBinGo, "cmd")
}
}

Expand Down

0 comments on commit 2719f41

Please sign in to comment.