Skip to content

Commit 2719f41

Browse files
Bryan C. Millsjohanbrandhorst
Bryan C. Mills
authored andcommitted
cmd/dist: leave cgo enabled if external linking is required
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]>
1 parent 1b4b504 commit 2719f41

File tree

3 files changed

+87
-33
lines changed

3 files changed

+87
-33
lines changed

src/cmd/dist/build.go

Lines changed: 59 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -55,6 +55,7 @@ var (
5555

5656
rebuildall bool
5757
noOpt bool
58+
isRelease bool
5859

5960
vflag int // verbosity
6061
)
@@ -257,6 +258,9 @@ func xinit() {
257258
xatexit(rmworkdir)
258259

259260
tooldir = pathf("%s/pkg/tool/%s_%s", goroot, gohostos, gohostarch)
261+
262+
goversion := findgoversion()
263+
isRelease = strings.HasPrefix(goversion, "release.") || strings.HasPrefix(goversion, "go")
260264
}
261265

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

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

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

542536
/*
543537
* Tool building
544538
*/
545539

540+
// mustLinkExternal is a copy of internal/platform.MustLinkExternal,
541+
// duplicated here to avoid version skew in the MustLinkExternal function
542+
// during bootstrapping.
543+
func mustLinkExternal(goos, goarch string) bool {
544+
switch goos {
545+
case "android":
546+
if goarch != "arm64" {
547+
return true
548+
}
549+
case "ios":
550+
if goarch == "arm64" {
551+
return true
552+
}
553+
}
554+
return false
555+
}
556+
546557
// deptab lists changes to the default dependencies for a given prefix.
547558
// deps ending in /* read the whole directory; deps beginning with -
548559
// exclude files with that prefix.
@@ -1266,14 +1277,33 @@ func timelog(op, name string) {
12661277
fmt.Fprintf(timeLogFile, "%s %+.1fs %s %s\n", t.Format(time.UnixDate), t.Sub(timeLogStart).Seconds(), op, name)
12671278
}
12681279

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

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

@@ -1414,7 +1444,7 @@ func cmdbootstrap() {
14141444
os.Setenv("CC", compilerEnvLookup("CC", defaultcc, goos, goarch))
14151445
// Now that cmd/go is in charge of the build process, enable GOEXPERIMENT.
14161446
os.Setenv("GOEXPERIMENT", goexperiment)
1417-
goInstall(toolenv, goBootstrap, toolchain...)
1447+
goInstall(toolenv(), goBootstrap, toolchain...)
14181448
if debug {
14191449
run("", ShowOutput|CheckExit, pathf("%s/compile", tooldir), "-V=full")
14201450
copyfile(pathf("%s/compile2", tooldir), pathf("%s/compile", tooldir), writeExec)
@@ -1441,7 +1471,7 @@ func cmdbootstrap() {
14411471
xprintf("\n")
14421472
}
14431473
xprintf("Building Go toolchain3 using go_bootstrap and Go toolchain2.\n")
1444-
goInstall(toolenv, goBootstrap, append([]string{"-a"}, toolchain...)...)
1474+
goInstall(toolenv(), goBootstrap, append([]string{"-a"}, toolchain...)...)
14451475
if debug {
14461476
run("", ShowOutput|CheckExit, pathf("%s/compile", tooldir), "-V=full")
14471477
copyfile(pathf("%s/compile3", tooldir), pathf("%s/compile", tooldir), writeExec)
@@ -1470,9 +1500,9 @@ func cmdbootstrap() {
14701500
xprintf("\n")
14711501
}
14721502
xprintf("Building commands for host, %s/%s.\n", goos, goarch)
1473-
goInstall(toolenv, goBootstrap, "cmd")
1474-
checkNotStale(toolenv, goBootstrap, "cmd")
1475-
checkNotStale(toolenv, gorootBinGo, "cmd")
1503+
goInstall(toolenv(), goBootstrap, "cmd")
1504+
checkNotStale(toolenv(), goBootstrap, "cmd")
1505+
checkNotStale(toolenv(), gorootBinGo, "cmd")
14761506

14771507
timelog("build", "target toolchain")
14781508
if vflag > 0 {
@@ -1486,15 +1516,15 @@ func cmdbootstrap() {
14861516
xprintf("Building packages and commands for target, %s/%s.\n", goos, goarch)
14871517
}
14881518
goInstall(nil, goBootstrap, "std")
1489-
goInstall(toolenv, goBootstrap, "cmd")
1490-
checkNotStale(toolenv, goBootstrap, append(toolchain, "runtime/internal/sys")...)
1519+
goInstall(toolenv(), goBootstrap, "cmd")
1520+
checkNotStale(toolenv(), goBootstrap, append(toolchain, "runtime/internal/sys")...)
14911521
checkNotStale(nil, goBootstrap, "std")
1492-
checkNotStale(toolenv, goBootstrap, "cmd")
1522+
checkNotStale(toolenv(), goBootstrap, "cmd")
14931523
checkNotStale(nil, gorootBinGo, "std")
1494-
checkNotStale(toolenv, gorootBinGo, "cmd")
1524+
checkNotStale(toolenv(), gorootBinGo, "cmd")
14951525
if debug {
14961526
run("", ShowOutput|CheckExit, pathf("%s/compile", tooldir), "-V=full")
1497-
checkNotStale(toolenv, goBootstrap, append(toolchain, "runtime/internal/sys")...)
1527+
checkNotStale(toolenv(), goBootstrap, append(toolchain, "runtime/internal/sys")...)
14981528
copyfile(pathf("%s/compile4", tooldir), pathf("%s/compile", tooldir), writeExec)
14991529
}
15001530

src/cmd/dist/build_test.go

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,24 @@
1+
// Copyright 2023 The Go Authors. All rights reserved.
2+
// Use of this source code is governed by a BSD-style
3+
// license that can be found in the LICENSE file.
4+
5+
package main
6+
7+
import (
8+
"internal/platform"
9+
"testing"
10+
)
11+
12+
// TestMustLinkExternal verifies that the mustLinkExternal helper
13+
// function matches internal/platform.MustLinkExternal.
14+
func TestMustLinkExternal(t *testing.T) {
15+
for _, goos := range okgoos {
16+
for _, goarch := range okgoarch {
17+
got := mustLinkExternal(goos, goarch)
18+
want := platform.MustLinkExternal(goos, goarch)
19+
if got != want {
20+
t.Errorf("mustLinkExternal(%q, %q) = %v; want %v", goos, goarch, got, want)
21+
}
22+
}
23+
}
24+
}

src/cmd/dist/test.go

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -149,7 +149,7 @@ func (t *tester) run() {
149149
if t.rebuild {
150150
t.out("Building packages and commands.")
151151
// Force rebuild the whole toolchain.
152-
goInstall(toolenv, gorootBinGo, append([]string{"-a"}, toolchain...)...)
152+
goInstall(toolenv(), gorootBinGo, append([]string{"-a"}, toolchain...)...)
153153
}
154154

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

0 commit comments

Comments
 (0)