Skip to content

Commit

Permalink
cmd/go: run tests when cmd/go is cross-compiled
Browse files Browse the repository at this point in the history
When the GOOS or GOARCH of the cmd/go test binary does not match the
GOOS or GOARCH of the installed 'go' binary itself, the test currently
attempts to trick 'go test' into thinking that there were no test
functions to run.

That makes it very difficult to discover how to actually run the
tests, which in turn makes it difficult to diagnose and fix
regressions in, say, the linux-386-longtest builders. (We have had a
few of those lately, and they shouldn't be as much of an ordeal to fix
as they currently are.)

There are three underlying problems:

1. cmd/go uses its own GOOS and GOARCH to figure out which variant of
   other tools to use, and the cache keys for all installed tools and
   libraries include the IDs of the tools used to build them. So when
   cmd/go's GOARCH changes, all installed tools and binaries appear
   stale *even if* they were just installed by invoking the native
   cmd/go with the appropriate GOARCH value set.

2. The "go/build" library used by cmd/go toggles its default
   CGO_ENABLED behavior depending on whether the GOOS and GOARCH being
   imported match runtime.GOOS and runtime.GOARCH.

3. A handful of cmd/go tests explicitly use gccgo, but the user's
   installed gccgo binary cannot necessarily cross-compile to the same
   platforms as cmd/go.

To address the cache-invalidation problem, we modify the test variant
of cmd/go to use the host's native toolchain (as indicated by the new
TESTGO_GOHOSTOS and TESTGO_GOHOSTARCH environment variables) instead
of the toolchain matching the test binary itself. That allows a test
cmd/go binary compiled with GOARCH=386 to use libraries and tools
cross-compiled by the native toolchain, so that

	$ GOARCH=386 go install std cmd

suffices to make the packages in std and cmd non-stale in the
tests.

To address the CGO_ENABLED mismatch, we set CGO_ENABLED explicitly in
the test's environment whenever it may differ from the default. Since
script tests that use cgo are already expected to use a [cgo]
condition, setting the environment to match that condition fixes the
cgo-specific tests.

To address the gccgo-specific cross-compilation failures, we add a new
script condition, [cross], which evaluates to true whenever the
platform of the test binary differs from that of the native toolchain.
We can then use that condition to explicitly skip the handful of gccgo
tests that fail under cross-compilation.

Fixes #53936.

Change-Id: I8633944f674eb5941ccc95df928991660e7e8137
Reviewed-on: https://go-review.googlesource.com/c/go/+/356611
Run-TryBot: Bryan Mills <[email protected]>
TryBot-Result: Gopher Robot <[email protected]>
Auto-Submit: Bryan Mills <[email protected]>
Reviewed-by: Russ Cox <[email protected]>
  • Loading branch information
Bryan C. Mills authored and gopherbot committed Aug 18, 2022
1 parent d8f90ce commit e64c871
Show file tree
Hide file tree
Showing 17 changed files with 120 additions and 76 deletions.
41 changes: 21 additions & 20 deletions src/cmd/go/go_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,11 @@ var (
fuzzInstrumented = false // whether fuzzing uses instrumentation
)

var (
goHostOS, goHostArch string
cgoEnabled string // raw value from 'go env CGO_ENABLED'
)

var exeSuffix string = func() string {
if runtime.GOOS == "windows" {
return ".exe"
Expand Down Expand Up @@ -96,6 +101,8 @@ func TestMain(m *testing.M) {
// run the main func exported via export_test.go, and exit.
// We set CMDGO_TEST_RUN_MAIN via os.Setenv and testScript.setup.
if os.Getenv("CMDGO_TEST_RUN_MAIN") != "" {
cfg.SetGOROOT(cfg.GOROOT, true)

if v := os.Getenv("TESTGO_VERSION"); v != "" {
work.RuntimeVersion = v
}
Expand Down Expand Up @@ -204,13 +211,16 @@ func TestMain(m *testing.M) {
// which will cause many tests to do unnecessary rebuilds and some
// tests to attempt to overwrite the installed standard library.
// Bail out entirely in this case.
hostGOOS := goEnv("GOHOSTOS")
hostGOARCH := goEnv("GOHOSTARCH")
if hostGOOS != runtime.GOOS || hostGOARCH != runtime.GOARCH {
fmt.Fprintf(os.Stderr, "testing: warning: no tests to run\n") // magic string for cmd/go
fmt.Printf("cmd/go test is not compatible with GOOS/GOARCH != GOHOSTOS/GOHOSTARCH (%s/%s != %s/%s)\n", runtime.GOOS, runtime.GOARCH, hostGOOS, hostGOARCH)
fmt.Printf("SKIP\n")
return
goHostOS = goEnv("GOHOSTOS")
os.Setenv("TESTGO_GOHOSTOS", goHostOS)
goHostArch = goEnv("GOHOSTARCH")
os.Setenv("TESTGO_GOHOSTARCH", goHostArch)

cgoEnabled = goEnv("CGO_ENABLED")
canCgo, err = strconv.ParseBool(cgoEnabled)
if err != nil {
fmt.Fprintf(os.Stderr, "can't parse go env CGO_ENABLED output: %q\n", strings.TrimSpace(cgoEnabled))
os.Exit(2)
}

// Duplicate the test executable into the path at testGo, for $PATH.
Expand Down Expand Up @@ -241,18 +251,6 @@ func TestMain(m *testing.M) {
}
}

cmd := exec.Command(testGo, "env", "CGO_ENABLED")
cmd.Stderr = new(strings.Builder)
if out, err := cmd.Output(); err != nil {
fmt.Fprintf(os.Stderr, "running testgo failed: %v\n%s", err, cmd.Stderr)
os.Exit(2)
} else {
canCgo, err = strconv.ParseBool(strings.TrimSpace(string(out)))
if err != nil {
fmt.Fprintf(os.Stderr, "can't parse go env CGO_ENABLED output: %v\n", strings.TrimSpace(string(out)))
}
}

out, err := exec.Command(gotool, "env", "GOCACHE").CombinedOutput()
if err != nil {
fmt.Fprintf(os.Stderr, "could not find testing GOCACHE: %v\n%s", err, out)
Expand All @@ -272,6 +270,7 @@ func TestMain(m *testing.M) {
canFuzz = sys.FuzzSupported(runtime.GOOS, runtime.GOARCH)
fuzzInstrumented = sys.FuzzInstrumented(runtime.GOOS, runtime.GOARCH)
}

// Don't let these environment variables confuse the test.
os.Setenv("GOENV", "off")
os.Unsetenv("GOFLAGS")
Expand Down Expand Up @@ -886,7 +885,7 @@ func TestNewReleaseRebuildsStalePackagesInGOPATH(t *testing.T) {
"src/math/bits",
"src/unsafe",
filepath.Join("pkg", runtime.GOOS+"_"+runtime.GOARCH),
filepath.Join("pkg/tool", runtime.GOOS+"_"+runtime.GOARCH),
filepath.Join("pkg/tool", goHostOS+"_"+goHostArch),
"pkg/include",
} {
srcdir := filepath.Join(testGOROOT, copydir)
Expand Down Expand Up @@ -2377,6 +2376,8 @@ func TestIssue22588(t *testing.T) {
defer tg.cleanup()
tg.parallel()

tg.wantNotStale("runtime", "", "must be non-stale to compare staleness under -toolexec")

if _, err := os.Stat("/usr/bin/time"); err != nil {
t.Skip(err)
}
Expand Down
16 changes: 1 addition & 15 deletions src/cmd/go/internal/base/tool.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,28 +9,14 @@ import (
"go/build"
"os"
"path/filepath"
"runtime"

"cmd/go/internal/cfg"
)

// Configuration for finding tool binaries.
var (
ToolGOOS = runtime.GOOS
ToolGOARCH = runtime.GOARCH
ToolIsWindows = ToolGOOS == "windows"
ToolDir = build.ToolDir
)

const ToolWindowsExtension = ".exe"

// Tool returns the path to the named tool (for example, "vet").
// If the tool cannot be found, Tool exits the process.
func Tool(toolName string) string {
toolPath := filepath.Join(ToolDir, toolName)
if ToolIsWindows {
toolPath += ToolWindowsExtension
}
toolPath := filepath.Join(build.ToolDir, toolName) + cfg.ToolExeSuffix()
if len(cfg.BuildToolexec) > 0 {
return toolPath
}
Expand Down
62 changes: 53 additions & 9 deletions src/cmd/go/internal/cfg/cfg.go
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,25 @@ func exeSuffix() string {
return ""
}

// Configuration for tools installed to GOROOT/bin.
// Normally these match runtime.GOOS and runtime.GOARCH,
// but when testing a cross-compiled cmd/go they will
// indicate the GOOS and GOARCH of the installed cmd/go
// rather than the test binary.
var (
installedGOOS string
installedGOARCH string
)

// ToolExeSuffix returns the suffix for executables installed
// in build.ToolDir.
func ToolExeSuffix() string {
if installedGOOS == "windows" {
return ".exe"
}
return ""
}

// These are general "build flags" used by build and other commands.
var (
BuildA bool // -a flag
Expand Down Expand Up @@ -141,12 +160,17 @@ func defaultContext() build.Context {
}

func init() {
SetGOROOT(findGOROOT())
SetGOROOT(findGOROOT(), false)
BuildToolchainCompiler = func() string { return "missing-compiler" }
BuildToolchainLinker = func() string { return "missing-linker" }
}

func SetGOROOT(goroot string) {
// SetGOROOT sets GOROOT and associated variables to the given values.
//
// If isTestGo is true, build.ToolDir is set based on the TESTGO_GOHOSTOS and
// TESTGO_GOHOSTARCH environment variables instead of runtime.GOOS and
// runtime.GOARCH.
func SetGOROOT(goroot string, isTestGo bool) {
BuildContext.GOROOT = goroot

GOROOT = goroot
Expand All @@ -161,13 +185,33 @@ func SetGOROOT(goroot string) {
}
GOROOT_FINAL = findGOROOT_FINAL(goroot)

if runtime.Compiler != "gccgo" && goroot != "" {
// Note that we must use runtime.GOOS and runtime.GOARCH here,
// as the tool directory does not move based on environment
// variables. This matches the initialization of ToolDir in
// go/build, except for using BuildContext.GOROOT rather than
// runtime.GOROOT.
build.ToolDir = filepath.Join(goroot, "pkg/tool/"+runtime.GOOS+"_"+runtime.GOARCH)
installedGOOS = runtime.GOOS
installedGOARCH = runtime.GOARCH
if isTestGo {
if testOS := os.Getenv("TESTGO_GOHOSTOS"); testOS != "" {
installedGOOS = testOS
}
if testArch := os.Getenv("TESTGO_GOHOSTARCH"); testArch != "" {
installedGOARCH = testArch
}
}

if runtime.Compiler != "gccgo" {
if goroot == "" {
build.ToolDir = ""
} else {
// Note that we must use the installed OS and arch here: the tool
// directory does not move based on environment variables, and even if we
// are testing a cross-compiled cmd/go all of the installed packages and
// tools would have been built using the native compiler and linker (and
// would spuriously appear stale if we used a cross-compiled compiler and
// linker).
//
// This matches the initialization of ToolDir in go/build, except for
// using ctxt.GOROOT and the installed GOOS and GOARCH rather than the
// GOROOT, GOOS, and GOARCH reported by the runtime package.
build.ToolDir = filepath.Join(GOROOTpkg, "tool", installedGOOS+"_"+installedGOARCH)
}
}
}

Expand Down
3 changes: 2 additions & 1 deletion src/cmd/go/internal/clean/clean.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import (
"io"
"os"
"path/filepath"
"runtime"
"strconv"
"strings"
"time"
Expand Down Expand Up @@ -395,7 +396,7 @@ func removeFile(f string) {
return
}
// Windows does not allow deletion of a binary file while it is executing.
if base.ToolIsWindows {
if runtime.GOOS == "windows" {
// Remove lingering ~ file from last attempt.
if _, err2 := os.Stat(f + "~"); err2 == nil {
os.Remove(f + "~")
Expand Down
2 changes: 1 addition & 1 deletion src/cmd/go/internal/envcmd/env.go
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,7 @@ func MkEnv() []cfg.EnvVar {
{Name: "GOROOT", Value: cfg.GOROOT},
{Name: "GOSUMDB", Value: cfg.GOSUMDB},
{Name: "GOTMPDIR", Value: cfg.Getenv("GOTMPDIR")},
{Name: "GOTOOLDIR", Value: base.ToolDir},
{Name: "GOTOOLDIR", Value: build.ToolDir},
{Name: "GOVCS", Value: cfg.GOVCS},
{Name: "GOVERSION", Value: runtime.Version()},
}
Expand Down
5 changes: 1 addition & 4 deletions src/cmd/go/internal/fmtcmd/fmt.go
Original file line number Diff line number Diff line change
Expand Up @@ -97,10 +97,7 @@ func runFmt(ctx context.Context, cmd *base.Command, args []string) {
}

func gofmtPath() string {
gofmt := "gofmt"
if base.ToolIsWindows {
gofmt += base.ToolWindowsExtension
}
gofmt := "gofmt" + cfg.ToolExeSuffix()

gofmtPath := filepath.Join(cfg.GOBIN, gofmt)
if _, err := os.Stat(gofmtPath); err == nil {
Expand Down
18 changes: 7 additions & 11 deletions src/cmd/go/internal/load/pkg.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@ import (
"io/fs"
"os"
"os/exec"
"path"
pathpkg "path"
"path/filepath"
"runtime"
Expand Down Expand Up @@ -1776,9 +1775,9 @@ func (p *Package) load(ctx context.Context, opts PackageOpts, path string, stk *
setError(e)
return
}
elem := p.DefaultExecName()
full := cfg.BuildContext.GOOS + "_" + cfg.BuildContext.GOARCH + "/" + elem
if cfg.BuildContext.GOOS != base.ToolGOOS || cfg.BuildContext.GOARCH != base.ToolGOARCH {
elem := p.DefaultExecName() + cfg.ExeSuffix
full := cfg.BuildContext.GOOS + "_" + cfg.BuildContext.GOARCH + string(filepath.Separator) + elem
if cfg.BuildContext.GOOS != runtime.GOOS || cfg.BuildContext.GOARCH != runtime.GOARCH {
// Install cross-compiled binaries to subdirectories of bin.
elem = full
}
Expand All @@ -1788,7 +1787,7 @@ func (p *Package) load(ctx context.Context, opts PackageOpts, path string, stk *
if p.Internal.Build.BinDir != "" {
// Install to GOBIN or bin of GOPATH entry.
p.Target = filepath.Join(p.Internal.Build.BinDir, elem)
if !p.Goroot && strings.Contains(elem, "/") && cfg.GOBIN != "" {
if !p.Goroot && strings.Contains(elem, string(filepath.Separator)) && cfg.GOBIN != "" {
// Do not create $GOBIN/goos_goarch/elem.
p.Target = ""
p.Internal.GobinSubdir = true
Expand All @@ -1798,14 +1797,11 @@ func (p *Package) load(ctx context.Context, opts PackageOpts, path string, stk *
// This is for 'go tool'.
// Override all the usual logic and force it into the tool directory.
if cfg.BuildToolchainName == "gccgo" {
p.Target = filepath.Join(base.ToolDir, elem)
p.Target = filepath.Join(build.ToolDir, elem)
} else {
p.Target = filepath.Join(cfg.GOROOTpkg, "tool", full)
}
}
if p.Target != "" && cfg.BuildContext.GOOS == "windows" {
p.Target += ".exe"
}
} else if p.Internal.Local {
// Local import turned into absolute path.
// No permanent install target.
Expand Down Expand Up @@ -2071,7 +2067,7 @@ func resolveEmbed(pkgdir string, patterns []string) (files []string, pmap map[st
glob = pattern[len("all:"):]
}
// Check pattern is valid for //go:embed.
if _, err := path.Match(glob, ""); err != nil || !validEmbedPattern(glob) {
if _, err := pathpkg.Match(glob, ""); err != nil || !validEmbedPattern(glob) {
return nil, nil, fmt.Errorf("invalid pattern syntax")
}

Expand Down Expand Up @@ -3112,7 +3108,7 @@ func PackagesAndErrorsOutsideModule(ctx context.Context, opts PackageOpts, args
return nil, fmt.Errorf("%s: argument must be a package path, not an absolute path", arg)
case search.IsMetaPackage(p):
return nil, fmt.Errorf("%s: argument must be a package path, not a meta-package", arg)
case path.Clean(p) != p:
case pathpkg.Clean(p) != p:
return nil, fmt.Errorf("%s: argument must be a clean package path", arg)
case !strings.Contains(p, "...") && search.IsStandardImportPath(p) && modindex.IsStandardPackage(cfg.GOROOT, cfg.BuildContext.Compiler, p):
return nil, fmt.Errorf("%s: argument must not be a package in the standard library", arg)
Expand Down
2 changes: 1 addition & 1 deletion src/cmd/go/internal/test/flagdefs_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ import (
)

func TestMain(m *testing.M) {
cfg.SetGOROOT(testenv.GOROOT(nil))
cfg.SetGOROOT(testenv.GOROOT(nil), false)
}

func TestPassFlagToTestIncludesAllTestFlags(t *testing.T) {
Expand Down
9 changes: 4 additions & 5 deletions src/cmd/go/internal/tool/tool.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ package tool
import (
"context"
"fmt"
"go/build"
"os"
"os/exec"
"os/signal"
Expand Down Expand Up @@ -115,7 +116,7 @@ func runTool(ctx context.Context, cmd *base.Command, args []string) {

// listTools prints a list of the available tools in the tools directory.
func listTools() {
f, err := os.Open(base.ToolDir)
f, err := os.Open(build.ToolDir)
if err != nil {
fmt.Fprintf(os.Stderr, "go: no tool directory: %s\n", err)
base.SetExitStatus(2)
Expand All @@ -132,11 +133,9 @@ func listTools() {
sort.Strings(names)
for _, name := range names {
// Unify presentation by going to lower case.
name = strings.ToLower(name)
// If it's windows, don't show the .exe suffix.
if base.ToolIsWindows && strings.HasSuffix(name, base.ToolWindowsExtension) {
name = name[:len(name)-len(base.ToolWindowsExtension)]
}
name = strings.TrimSuffix(strings.ToLower(name), cfg.ToolExeSuffix())

// The tool directory used by gccgo will have other binaries
// in addition to go tools. Only display go tools here.
if cfg.BuildToolchainName == "gccgo" && !isGccgoTool(name) {
Expand Down
4 changes: 2 additions & 2 deletions src/cmd/go/internal/work/exec.go
Original file line number Diff line number Diff line change
Expand Up @@ -1813,15 +1813,15 @@ func (b *Builder) copyFile(dst, src string, perm fs.FileMode, force bool) error
}

// On Windows, remove lingering ~ file from last attempt.
if base.ToolIsWindows {
if runtime.GOOS == "windows" {
if _, err := os.Stat(dst + "~"); err == nil {
os.Remove(dst + "~")
}
}

mayberemovefile(dst)
df, err := os.OpenFile(dst, os.O_WRONLY|os.O_CREATE|os.O_TRUNC, perm)
if err != nil && base.ToolIsWindows {
if err != nil && runtime.GOOS == "windows" {
// Windows does not allow deletion of a binary file
// while it is executing. Try to move it out of the way.
// If the move fails, which is likely, we'll try again the
Expand Down
Loading

0 comments on commit e64c871

Please sign in to comment.