internal/testenv: avoid rebuilding all of std in WriteImportcfg

Instead, have the caller pass in an explicit list of the packages
(if any) they need.

After #47257, a builder running a test does not necessarily have the
entire standard library already cached, especially when running tests
in sharded mode. testenv.WriteImportcfg used to write an importcfg for
the entire standard library — which required rebuilding the entire
standard library — even though most tests need only a tiny subset.

This reduces the time to test internal/abi with a cold build cache on
my workstation from ~16s to ~0.05s.

It somewhat increases the time for 'go test go/internal/gcimporter'
with a cold cache, from ~43s to ~54s, presumably due to decreased
parallelism in rebuilding the standard library and increased overhead
in re-resolving the import map. However, 'go test -short' running time
remains stable (~5.5s before and after).

Fixes #58248.

Change-Id: I9be6b61ae6e28b75b53af85207c281bb93b9346f
Reviewed-on: https://go-review.googlesource.com/c/go/+/464736
Run-TryBot: Bryan Mills <bcmills@google.com>
Reviewed-by: Cherry Mui <cherryyz@google.com>
Reviewed-by: Than McIntosh <thanm@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Auto-Submit: Bryan Mills <bcmills@google.com>

diff --git a/src/cmd/internal/archive/archive_test.go b/src/cmd/internal/archive/archive_test.go
index 0e2c7bca75e0eecd013929dc8a7455a49ab09129..10a3d6ebeb65e458bcfcd01640f758cd7b188a5e 100644 (file)
--- a/src/cmd/internal/archive/archive_test.go
+++ b/src/cmd/internal/archive/archive_test.go
@@ -113,7 +113,7 @@ func buildGoobj(t *testing.T) goobjPaths {
                        go2src := filepath.Join("testdata", "go2.go")
 
                        importcfgfile := filepath.Join(buildDir, "importcfg")
-                       testenv.WriteImportcfg(t, importcfgfile, nil)
+                       testenv.WriteImportcfg(t, importcfgfile, nil, go1src, go2src)
 
                        out, err := testenv.Command(t, gotool, "tool", "compile", "-importcfg="+importcfgfile, "-p=p", "-o", go1obj, go1src).CombinedOutput()
                        if err != nil {

diff --git a/src/cmd/link/link_test.go b/src/cmd/link/link_test.go
index 4dca2e20d61c5a4927e6964bdd343d0c3a3a5163..b4ef9ada17c6826bba48df8d9406175f94dba2d2 100644 (file)
--- a/src/cmd/link/link_test.go
+++ b/src/cmd/link/link_test.go
@@ -49,15 +49,16 @@ func main() {}
 `
 
        tmpdir := t.TempDir()
+       main := filepath.Join(tmpdir, "main.go")
 
-       importcfgfile := filepath.Join(tmpdir, "importcfg")
-       testenv.WriteImportcfg(t, importcfgfile, nil)
-
-       err := os.WriteFile(filepath.Join(tmpdir, "main.go"), []byte(source), 0666)
+       err := os.WriteFile(main, []byte(source), 0666)
        if err != nil {
                t.Fatalf("failed to write main.go: %v\n", err)
        }
 
+       importcfgfile := filepath.Join(tmpdir, "importcfg")
+       testenv.WriteImportcfg(t, importcfgfile, nil, main)
+
        cmd := testenv.Command(t, testenv.GoToolPath(t), "tool", "compile", "-importcfg="+importcfgfile, "-p=main", "main.go")
        cmd.Dir = tmpdir
        out, err := cmd.CombinedOutput()
@@ -101,11 +102,10 @@ func TestIssue28429(t *testing.T) {
                }
        }
 
-       importcfgfile := filepath.Join(tmpdir, "importcfg")
-       testenv.WriteImportcfg(t, importcfgfile, nil)
-
        // Compile a main package.
        write("main.go", "package main; func main() {}")
+       importcfgfile := filepath.Join(tmpdir, "importcfg")
+       testenv.WriteImportcfg(t, importcfgfile, nil, filepath.Join(tmpdir, "main.go"))
        runGo("tool", "compile", "-importcfg="+importcfgfile, "-p=main", "main.go")
        runGo("tool", "pack", "c", "main.a", "main.o")
 
@@ -243,7 +243,7 @@ void foo() {
        cflags := strings.Fields(runGo("env", "GOGCCFLAGS"))
 
        importcfgfile := filepath.Join(tmpdir, "importcfg")
-       testenv.WriteImportcfg(t, importcfgfile, nil)
+       testenv.WriteImportcfg(t, importcfgfile, nil, "runtime")
 
        // Compile, assemble and pack the Go and C code.
        runGo("tool", "asm", "-p=main", "-gensymabis", "-o", "symabis", "x.s")
@@ -787,10 +787,10 @@ func TestIndexMismatch(t *testing.T) {
        mObj := filepath.Join(tmpdir, "main.o")
        exe := filepath.Join(tmpdir, "main.exe")
 
-       importcfgFile := filepath.Join(tmpdir, "stdlib.importcfg")
-       testenv.WriteImportcfg(t, importcfgFile, nil)
+       importcfgFile := filepath.Join(tmpdir, "runtime.importcfg")
+       testenv.WriteImportcfg(t, importcfgFile, nil, "runtime")
        importcfgWithAFile := filepath.Join(tmpdir, "witha.importcfg")
-       testenv.WriteImportcfg(t, importcfgWithAFile, map[string]string{"a": aObj})
+       testenv.WriteImportcfg(t, importcfgWithAFile, map[string]string{"a": aObj}, "runtime")
 
        // Build a program with main package importing package a.
        cmd := testenv.Command(t, testenv.GoToolPath(t), "tool", "compile", "-importcfg="+importcfgFile, "-p=a", "-o", aObj, aSrc)

diff --git a/src/cmd/objdump/objdump_test.go b/src/cmd/objdump/objdump_test.go
index 69b4cf4e212ef916a9bc5c213cc111d4d9126c35..226e74d81e8e1e1928348c8c93472c4915aecde9 100644 (file)
--- a/src/cmd/objdump/objdump_test.go
+++ b/src/cmd/objdump/objdump_test.go
@@ -299,7 +299,7 @@ func TestDisasmGoobj(t *testing.T) {
        tmp := t.TempDir()
 
        importcfgfile := filepath.Join(tmp, "hello.importcfg")
-       testenv.WriteImportcfg(t, importcfgfile, nil)
+       testenv.WriteImportcfg(t, importcfgfile, nil, "testdata/fmthello.go")
 
        hello := filepath.Join(tmp, "hello.o")
        args := []string{"tool", "compile", "-p=main", "-importcfg=" + importcfgfile, "-o", hello}

diff --git a/src/cmd/pack/pack_test.go b/src/cmd/pack/pack_test.go
index be75738093512b8f34a4b55b6917d215b65e19d2..5534a10b37f3cd28b5d2d40cc77a04b105ca8195 100644 (file)
--- a/src/cmd/pack/pack_test.go
+++ b/src/cmd/pack/pack_test.go
@@ -210,7 +210,7 @@ func TestHello(t *testing.T) {
        }
 
        importcfgfile := filepath.Join(dir, "hello.importcfg")
-       testenv.WriteImportcfg(t, importcfgfile, nil)
+       testenv.WriteImportcfg(t, importcfgfile, nil, hello)
 
        goBin := testenv.GoToolPath(t)
        run(goBin, "tool", "compile", "-importcfg="+importcfgfile, "-p=main", "hello.go")
@@ -284,7 +284,7 @@ func TestLargeDefs(t *testing.T) {
        goBin := testenv.GoToolPath(t)
        run(goBin, "tool", "compile", "-importcfg="+importcfgfile, "-p=large", "large.go")
        run(packPath(t), "grc", "large.a", "large.o")
-       testenv.WriteImportcfg(t, importcfgfile, map[string]string{"large": filepath.Join(dir, "large.o")})
+       testenv.WriteImportcfg(t, importcfgfile, map[string]string{"large": filepath.Join(dir, "large.o")}, "runtime")
        run(goBin, "tool", "compile", "-importcfg="+importcfgfile, "-p=main", "main.go")
        run(goBin, "tool", "link", "-importcfg="+importcfgfile, "-L", ".", "-o", "a.out", "main.o")
        out := run("./a.out")

diff --git a/src/go/internal/gcimporter/gcimporter_test.go b/src/go/internal/gcimporter/gcimporter_test.go
index 3270f3d68292a57b85c73d914a191d1b25f8eb7c..800c372971acc3441f23a5a3492891be63888596 100644 (file)
--- a/src/go/internal/gcimporter/gcimporter_test.go
+++ b/src/go/internal/gcimporter/gcimporter_test.go
@@ -7,7 +7,6 @@ package gcimporter_test
 import (
        "bytes"
        "fmt"
-       "internal/goroot"
        "internal/testenv"
        "os"
        "os/exec"
@@ -36,7 +35,7 @@ func TestMain(m *testing.M) {
 // compile runs the compiler on filename, with dirname as the working directory,
 // and writes the output file to outdirname.
 // compile gives the resulting package a packagepath of testdata/<filebasename>.
-func compile(t *testing.T, dirname, filename, outdirname string, packagefiles map[string]string) string {
+func compile(t *testing.T, dirname, filename, outdirname string, packageFiles map[string]string, pkgImports ...string) string {
        // filename must end with ".go"
        basename, ok := strings.CutSuffix(filepath.Base(filename), ".go")
        if !ok {
@@ -46,16 +45,9 @@ func compile(t *testing.T, dirname, filename, outdirname string, packagefiles ma
        outname := filepath.Join(outdirname, objname)
 
        importcfgfile := os.DevNull
-       if len(packagefiles) > 0 {
+       if len(packageFiles) > 0 || len(pkgImports) > 0 {
                importcfgfile = filepath.Join(outdirname, basename) + ".importcfg"
-               importcfg := new(bytes.Buffer)
-               fmt.Fprintf(importcfg, "# import config")
-               for k, v := range packagefiles {
-                       fmt.Fprintf(importcfg, "\npackagefile %s=%s\n", k, v)
-               }
-               if err := os.WriteFile(importcfgfile, importcfg.Bytes(), 0655); err != nil {
-                       t.Fatal(err)
-               }
+               testenv.WriteImportcfg(t, importcfgfile, packageFiles, pkgImports...)
        }
 
        pkgpath := path.Join("testdata", basename)
@@ -118,16 +110,7 @@ func TestImportTestdata(t *testing.T) {
                tmpdir := mktmpdir(t)
                defer os.RemoveAll(tmpdir)
 
-               packageFiles := map[string]string{}
-               for _, pkg := range wantImports {
-                       export, _ := FindPkg(pkg, "testdata")
-                       if export == "" {
-                               t.Fatalf("no export data found for %s", pkg)
-                       }
-                       packageFiles[pkg] = export
-               }
-
-               compile(t, "testdata", testfile, filepath.Join(tmpdir, "testdata"), packageFiles)
+               compile(t, "testdata", testfile, filepath.Join(tmpdir, "testdata"), nil, wantImports...)
                path := "./testdata/" + strings.TrimSuffix(testfile, ".go")
 
                if pkg := testPath(t, path, tmpdir); pkg != nil {
@@ -188,11 +171,7 @@ func TestImportTypeparamTests(t *testing.T) {
 
                        // Compile and import, and compare the resulting package with the package
                        // that was type-checked directly.
-                       pkgFiles, err := goroot.PkgfileMap()
-                       if err != nil {
-                               t.Fatal(err)
-                       }
-                       compile(t, rootDir, entry.Name(), filepath.Join(tmpdir, "testdata"), pkgFiles)
+                       compile(t, rootDir, entry.Name(), filepath.Join(tmpdir, "testdata"), nil, filename)
                        pkgName := strings.TrimSuffix(entry.Name(), ".go")
                        imported := importPkg(t, "./testdata/"+pkgName, tmpdir)
                        checked := checkFile(t, filename, src)
@@ -569,13 +548,8 @@ func TestIssue13566(t *testing.T) {
                t.Fatal(err)
        }
 
-       jsonExport, _ := FindPkg("encoding/json", "testdata")
-       if jsonExport == "" {
-               t.Fatalf("no export data found for encoding/json")
-       }
-
-       compile(t, "testdata", "a.go", testoutdir, map[string]string{"encoding/json": jsonExport})
-       compile(t, testoutdir, bpath, testoutdir, map[string]string{"testdata/a": filepath.Join(testoutdir, "a.o")})
+       compile(t, "testdata", "a.go", testoutdir, nil, "encoding/json")
+       compile(t, testoutdir, bpath, testoutdir, map[string]string{"testdata/a": filepath.Join(testoutdir, "a.o")}, "encoding/json")
 
        // import must succeed (test for issue at hand)
        pkg := importPkg(t, "./testdata/b", tmpdir)

diff --git a/src/internal/abi/abi_test.go b/src/internal/abi/abi_test.go
index f0d8dceb3ec9c14b9970afc9389ccd800647d768..44b9e78a304181d48faa6f23546f179307b7c8d2 100644 (file)
--- a/src/internal/abi/abi_test.go
+++ b/src/internal/abi/abi_test.go
@@ -7,7 +7,6 @@ package abi_test
 import (
        "internal/abi"
        "internal/testenv"
-       "os/exec"
        "path/filepath"
        "strings"
        "testing"
@@ -42,18 +41,19 @@ func TestFuncPCCompileError(t *testing.T) {
        symabi := filepath.Join(tmpdir, "symabi")
        obj := filepath.Join(tmpdir, "x.o")
 
+       // Write an importcfg file for the dependencies of the package.
        importcfgfile := filepath.Join(tmpdir, "hello.importcfg")
-       testenv.WriteImportcfg(t, importcfgfile, nil)
+       testenv.WriteImportcfg(t, importcfgfile, nil, "internal/abi")
 
        // parse assembly code for symabi.
-       cmd := exec.Command(testenv.GoToolPath(t), "tool", "asm", "-gensymabis", "-o", symabi, asmSrc)
+       cmd := testenv.Command(t, testenv.GoToolPath(t), "tool", "asm", "-gensymabis", "-o", symabi, asmSrc)
        out, err := cmd.CombinedOutput()
        if err != nil {
                t.Fatalf("go tool asm -gensymabis failed: %v\n%s", err, out)
        }
 
        // compile go code.
-       cmd = exec.Command(testenv.GoToolPath(t), "tool", "compile", "-importcfg="+importcfgfile, "-p=p", "-symabis", symabi, "-o", obj, goSrc)
+       cmd = testenv.Command(t, testenv.GoToolPath(t), "tool", "compile", "-importcfg="+importcfgfile, "-p=p", "-symabis", symabi, "-o", obj, goSrc)
        out, err = cmd.CombinedOutput()
        if err == nil {
                t.Fatalf("go tool compile did not fail")

diff --git a/src/internal/goroot/importcfg.go b/src/internal/goroot/importcfg.go
deleted file mode 100644 (file)
index e324073..0000000
--- a/src/internal/goroot/importcfg.go
+++ /dev/null
@@ -1,66 +0,0 @@
-// Copyright 2022 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 goroot
-
-import (
-       "bytes"
-       "fmt"
-       "os/exec"
-       "strings"
-       "sync"
-)
-
-// Importcfg returns an importcfg file to be passed to the
-// Go compiler that contains the cached paths for the .a files for the
-// standard library.
-func Importcfg() (string, error) {
-       var icfg bytes.Buffer
-
-       m, err := PkgfileMap()
-       if err != nil {
-               return "", err
-       }
-       fmt.Fprintf(&icfg, "# import config")
-       for importPath, export := range m {
-               fmt.Fprintf(&icfg, "\npackagefile %s=%s", importPath, export)
-       }
-       s := icfg.String()
-       return s, nil
-}
-
-var (
-       stdlibPkgfileMap map[string]string
-       stdlibPkgfileErr error
-       once             sync.Once
-)
-
-// PkgfileMap returns a map of package paths to the location on disk
-// of the .a file for the package.
-// The caller must not modify the map.
-func PkgfileMap() (map[string]string, error) {
-       once.Do(func() {
-               m := make(map[string]string)
-               output, err := exec.Command("go", "list", "-export", "-e", "-f", "{{.ImportPath}} {{.Export}}", "std", "cmd").Output()
-               if err != nil {
-                       stdlibPkgfileErr = err
-               }
-               for _, line := range strings.Split(string(output), "\n") {
-                       if line == "" {
-                               continue
-                       }
-                       sp := strings.SplitN(line, " ", 2)
-                       if len(sp) != 2 {
-                               stdlibPkgfileErr = fmt.Errorf("determining pkgfile map: invalid line in go list output: %q", line)
-                               return
-                       }
-                       importPath, export := sp[0], sp[1]
-                       if export != "" {
-                               m[importPath] = export
-                       }
-               }
-               stdlibPkgfileMap = m
-       })
-       return stdlibPkgfileMap, stdlibPkgfileErr
-}

diff --git a/src/internal/testenv/testenv.go b/src/internal/testenv/testenv.go
index 6a28b25278e87c4cca993af79398d08149347918..82fdfb6ff6e6b3cd328c82783141577426163969 100644 (file)
--- a/src/internal/testenv/testenv.go
+++ b/src/internal/testenv/testenv.go
@@ -11,11 +11,11 @@
 package testenv
 
 import (
+       "bytes"
        "errors"
        "flag"
        "fmt"
        "internal/cfg"
-       "internal/goroot"
        "internal/platform"
        "os"
        "os/exec"
@@ -347,18 +347,45 @@ func SkipIfOptimizationOff(t testing.TB) {
 }
 
 // WriteImportcfg writes an importcfg file used by the compiler or linker to
-// dstPath containing entries for the packages in std and cmd in addition
-// to the package to package file mappings in additionalPackageFiles.
-func WriteImportcfg(t testing.TB, dstPath string, additionalPackageFiles map[string]string) {
-       importcfg, err := goroot.Importcfg()
-       for k, v := range additionalPackageFiles {
-               importcfg += fmt.Sprintf("\npackagefile %s=%s", k, v)
+// dstPath containing entries for the file mappings in packageFiles, as well
+// as for the packages transitively imported by the package(s) in pkgs.
+//
+// pkgs may include any package pattern that is valid to pass to 'go list',
+// so it may also be a list of Go source files all in the same directory.
+func WriteImportcfg(t testing.TB, dstPath string, packageFiles map[string]string, pkgs ...string) {
+       t.Helper()
+
+       icfg := new(bytes.Buffer)
+       icfg.WriteString("# import config\n")
+       for k, v := range packageFiles {
+               fmt.Fprintf(icfg, "packagefile %s=%s\n", k, v)
        }
-       if err != nil {
-               t.Fatalf("preparing the importcfg failed: %s", err)
+
+       if len(pkgs) > 0 {
+               // Use 'go list' to resolve any missing packages and rewrite the import map.
+               cmd := Command(t, GoToolPath(t), "list", "-export", "-deps", "-f", `{{if ne .ImportPath "command-line-arguments"}}{{if .Export}}{{.ImportPath}}={{.Export}}{{end}}{{end}}`)
+               cmd.Args = append(cmd.Args, pkgs...)
+               cmd.Stderr = new(strings.Builder)
+               out, err := cmd.Output()
+               if err != nil {
+                       t.Fatalf("%v: %v\n%s", cmd, err, cmd.Stderr)
+               }
+
+               for _, line := range strings.Split(string(out), "\n") {
+                       if line == "" {
+                               continue
+                       }
+                       importPath, export, ok := strings.Cut(line, "=")
+                       if !ok {
+                               t.Fatalf("invalid line in output from %v:\n%s", cmd, line)
+                       }
+                       if packageFiles[importPath] == "" {
+                               fmt.Fprintf(icfg, "packagefile %s=%s\n", importPath, export)
+                       }
+               }
        }
-       err = os.WriteFile(dstPath, []byte(importcfg), 0655)
-       if err != nil {
-               t.Fatalf("writing the importcfg failed: %s", err)
+
+       if err := os.WriteFile(dstPath, icfg.Bytes(), 0666); err != nil {
+               t.Fatal(err)
        }
 }