Skip to content

Commit 4b43d66

Browse files
Bryan C. Millsgopherbot
Bryan C. Mills
authored andcommitted
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 <[email protected]> Reviewed-by: Cherry Mui <[email protected]> Reviewed-by: Than McIntosh <[email protected]> TryBot-Result: Gopher Robot <[email protected]> Auto-Submit: Bryan Mills <[email protected]>
1 parent 1877291 commit 4b43d66

File tree

8 files changed

+65
-130
lines changed

8 files changed

+65
-130
lines changed

src/cmd/internal/archive/archive_test.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -113,7 +113,7 @@ func buildGoobj(t *testing.T) goobjPaths {
113113
go2src := filepath.Join("testdata", "go2.go")
114114

115115
importcfgfile := filepath.Join(buildDir, "importcfg")
116-
testenv.WriteImportcfg(t, importcfgfile, nil)
116+
testenv.WriteImportcfg(t, importcfgfile, nil, go1src, go2src)
117117

118118
out, err := testenv.Command(t, gotool, "tool", "compile", "-importcfg="+importcfgfile, "-p=p", "-o", go1obj, go1src).CombinedOutput()
119119
if err != nil {

src/cmd/link/link_test.go

+11-11
Original file line numberDiff line numberDiff line change
@@ -49,15 +49,16 @@ func main() {}
4949
`
5050

5151
tmpdir := t.TempDir()
52+
main := filepath.Join(tmpdir, "main.go")
5253

53-
importcfgfile := filepath.Join(tmpdir, "importcfg")
54-
testenv.WriteImportcfg(t, importcfgfile, nil)
55-
56-
err := os.WriteFile(filepath.Join(tmpdir, "main.go"), []byte(source), 0666)
54+
err := os.WriteFile(main, []byte(source), 0666)
5755
if err != nil {
5856
t.Fatalf("failed to write main.go: %v\n", err)
5957
}
6058

59+
importcfgfile := filepath.Join(tmpdir, "importcfg")
60+
testenv.WriteImportcfg(t, importcfgfile, nil, main)
61+
6162
cmd := testenv.Command(t, testenv.GoToolPath(t), "tool", "compile", "-importcfg="+importcfgfile, "-p=main", "main.go")
6263
cmd.Dir = tmpdir
6364
out, err := cmd.CombinedOutput()
@@ -101,11 +102,10 @@ func TestIssue28429(t *testing.T) {
101102
}
102103
}
103104

104-
importcfgfile := filepath.Join(tmpdir, "importcfg")
105-
testenv.WriteImportcfg(t, importcfgfile, nil)
106-
107105
// Compile a main package.
108106
write("main.go", "package main; func main() {}")
107+
importcfgfile := filepath.Join(tmpdir, "importcfg")
108+
testenv.WriteImportcfg(t, importcfgfile, nil, filepath.Join(tmpdir, "main.go"))
109109
runGo("tool", "compile", "-importcfg="+importcfgfile, "-p=main", "main.go")
110110
runGo("tool", "pack", "c", "main.a", "main.o")
111111

@@ -243,7 +243,7 @@ void foo() {
243243
cflags := strings.Fields(runGo("env", "GOGCCFLAGS"))
244244

245245
importcfgfile := filepath.Join(tmpdir, "importcfg")
246-
testenv.WriteImportcfg(t, importcfgfile, nil)
246+
testenv.WriteImportcfg(t, importcfgfile, nil, "runtime")
247247

248248
// Compile, assemble and pack the Go and C code.
249249
runGo("tool", "asm", "-p=main", "-gensymabis", "-o", "symabis", "x.s")
@@ -787,10 +787,10 @@ func TestIndexMismatch(t *testing.T) {
787787
mObj := filepath.Join(tmpdir, "main.o")
788788
exe := filepath.Join(tmpdir, "main.exe")
789789

790-
importcfgFile := filepath.Join(tmpdir, "stdlib.importcfg")
791-
testenv.WriteImportcfg(t, importcfgFile, nil)
790+
importcfgFile := filepath.Join(tmpdir, "runtime.importcfg")
791+
testenv.WriteImportcfg(t, importcfgFile, nil, "runtime")
792792
importcfgWithAFile := filepath.Join(tmpdir, "witha.importcfg")
793-
testenv.WriteImportcfg(t, importcfgWithAFile, map[string]string{"a": aObj})
793+
testenv.WriteImportcfg(t, importcfgWithAFile, map[string]string{"a": aObj}, "runtime")
794794

795795
// Build a program with main package importing package a.
796796
cmd := testenv.Command(t, testenv.GoToolPath(t), "tool", "compile", "-importcfg="+importcfgFile, "-p=a", "-o", aObj, aSrc)

src/cmd/objdump/objdump_test.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -299,7 +299,7 @@ func TestDisasmGoobj(t *testing.T) {
299299
tmp := t.TempDir()
300300

301301
importcfgfile := filepath.Join(tmp, "hello.importcfg")
302-
testenv.WriteImportcfg(t, importcfgfile, nil)
302+
testenv.WriteImportcfg(t, importcfgfile, nil, "testdata/fmthello.go")
303303

304304
hello := filepath.Join(tmp, "hello.o")
305305
args := []string{"tool", "compile", "-p=main", "-importcfg=" + importcfgfile, "-o", hello}

src/cmd/pack/pack_test.go

+2-2
Original file line numberDiff line numberDiff line change
@@ -210,7 +210,7 @@ func TestHello(t *testing.T) {
210210
}
211211

212212
importcfgfile := filepath.Join(dir, "hello.importcfg")
213-
testenv.WriteImportcfg(t, importcfgfile, nil)
213+
testenv.WriteImportcfg(t, importcfgfile, nil, hello)
214214

215215
goBin := testenv.GoToolPath(t)
216216
run(goBin, "tool", "compile", "-importcfg="+importcfgfile, "-p=main", "hello.go")
@@ -284,7 +284,7 @@ func TestLargeDefs(t *testing.T) {
284284
goBin := testenv.GoToolPath(t)
285285
run(goBin, "tool", "compile", "-importcfg="+importcfgfile, "-p=large", "large.go")
286286
run(packPath(t), "grc", "large.a", "large.o")
287-
testenv.WriteImportcfg(t, importcfgfile, map[string]string{"large": filepath.Join(dir, "large.o")})
287+
testenv.WriteImportcfg(t, importcfgfile, map[string]string{"large": filepath.Join(dir, "large.o")}, "runtime")
288288
run(goBin, "tool", "compile", "-importcfg="+importcfgfile, "-p=main", "main.go")
289289
run(goBin, "tool", "link", "-importcfg="+importcfgfile, "-L", ".", "-o", "a.out", "main.o")
290290
out := run("./a.out")

src/go/internal/gcimporter/gcimporter_test.go

+7-33
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,6 @@ package gcimporter_test
77
import (
88
"bytes"
99
"fmt"
10-
"internal/goroot"
1110
"internal/testenv"
1211
"os"
1312
"os/exec"
@@ -36,7 +35,7 @@ func TestMain(m *testing.M) {
3635
// compile runs the compiler on filename, with dirname as the working directory,
3736
// and writes the output file to outdirname.
3837
// compile gives the resulting package a packagepath of testdata/<filebasename>.
39-
func compile(t *testing.T, dirname, filename, outdirname string, packagefiles map[string]string) string {
38+
func compile(t *testing.T, dirname, filename, outdirname string, packageFiles map[string]string, pkgImports ...string) string {
4039
// filename must end with ".go"
4140
basename, ok := strings.CutSuffix(filepath.Base(filename), ".go")
4241
if !ok {
@@ -46,16 +45,9 @@ func compile(t *testing.T, dirname, filename, outdirname string, packagefiles ma
4645
outname := filepath.Join(outdirname, objname)
4746

4847
importcfgfile := os.DevNull
49-
if len(packagefiles) > 0 {
48+
if len(packageFiles) > 0 || len(pkgImports) > 0 {
5049
importcfgfile = filepath.Join(outdirname, basename) + ".importcfg"
51-
importcfg := new(bytes.Buffer)
52-
fmt.Fprintf(importcfg, "# import config")
53-
for k, v := range packagefiles {
54-
fmt.Fprintf(importcfg, "\npackagefile %s=%s\n", k, v)
55-
}
56-
if err := os.WriteFile(importcfgfile, importcfg.Bytes(), 0655); err != nil {
57-
t.Fatal(err)
58-
}
50+
testenv.WriteImportcfg(t, importcfgfile, packageFiles, pkgImports...)
5951
}
6052

6153
pkgpath := path.Join("testdata", basename)
@@ -118,16 +110,7 @@ func TestImportTestdata(t *testing.T) {
118110
tmpdir := mktmpdir(t)
119111
defer os.RemoveAll(tmpdir)
120112

121-
packageFiles := map[string]string{}
122-
for _, pkg := range wantImports {
123-
export, _ := FindPkg(pkg, "testdata")
124-
if export == "" {
125-
t.Fatalf("no export data found for %s", pkg)
126-
}
127-
packageFiles[pkg] = export
128-
}
129-
130-
compile(t, "testdata", testfile, filepath.Join(tmpdir, "testdata"), packageFiles)
113+
compile(t, "testdata", testfile, filepath.Join(tmpdir, "testdata"), nil, wantImports...)
131114
path := "./testdata/" + strings.TrimSuffix(testfile, ".go")
132115

133116
if pkg := testPath(t, path, tmpdir); pkg != nil {
@@ -188,11 +171,7 @@ func TestImportTypeparamTests(t *testing.T) {
188171

189172
// Compile and import, and compare the resulting package with the package
190173
// that was type-checked directly.
191-
pkgFiles, err := goroot.PkgfileMap()
192-
if err != nil {
193-
t.Fatal(err)
194-
}
195-
compile(t, rootDir, entry.Name(), filepath.Join(tmpdir, "testdata"), pkgFiles)
174+
compile(t, rootDir, entry.Name(), filepath.Join(tmpdir, "testdata"), nil, filename)
196175
pkgName := strings.TrimSuffix(entry.Name(), ".go")
197176
imported := importPkg(t, "./testdata/"+pkgName, tmpdir)
198177
checked := checkFile(t, filename, src)
@@ -569,13 +548,8 @@ func TestIssue13566(t *testing.T) {
569548
t.Fatal(err)
570549
}
571550

572-
jsonExport, _ := FindPkg("encoding/json", "testdata")
573-
if jsonExport == "" {
574-
t.Fatalf("no export data found for encoding/json")
575-
}
576-
577-
compile(t, "testdata", "a.go", testoutdir, map[string]string{"encoding/json": jsonExport})
578-
compile(t, testoutdir, bpath, testoutdir, map[string]string{"testdata/a": filepath.Join(testoutdir, "a.o")})
551+
compile(t, "testdata", "a.go", testoutdir, nil, "encoding/json")
552+
compile(t, testoutdir, bpath, testoutdir, map[string]string{"testdata/a": filepath.Join(testoutdir, "a.o")}, "encoding/json")
579553

580554
// import must succeed (test for issue at hand)
581555
pkg := importPkg(t, "./testdata/b", tmpdir)

src/internal/abi/abi_test.go

+4-4
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,6 @@ package abi_test
77
import (
88
"internal/abi"
99
"internal/testenv"
10-
"os/exec"
1110
"path/filepath"
1211
"strings"
1312
"testing"
@@ -42,18 +41,19 @@ func TestFuncPCCompileError(t *testing.T) {
4241
symabi := filepath.Join(tmpdir, "symabi")
4342
obj := filepath.Join(tmpdir, "x.o")
4443

44+
// Write an importcfg file for the dependencies of the package.
4545
importcfgfile := filepath.Join(tmpdir, "hello.importcfg")
46-
testenv.WriteImportcfg(t, importcfgfile, nil)
46+
testenv.WriteImportcfg(t, importcfgfile, nil, "internal/abi")
4747

4848
// parse assembly code for symabi.
49-
cmd := exec.Command(testenv.GoToolPath(t), "tool", "asm", "-gensymabis", "-o", symabi, asmSrc)
49+
cmd := testenv.Command(t, testenv.GoToolPath(t), "tool", "asm", "-gensymabis", "-o", symabi, asmSrc)
5050
out, err := cmd.CombinedOutput()
5151
if err != nil {
5252
t.Fatalf("go tool asm -gensymabis failed: %v\n%s", err, out)
5353
}
5454

5555
// compile go code.
56-
cmd = exec.Command(testenv.GoToolPath(t), "tool", "compile", "-importcfg="+importcfgfile, "-p=p", "-symabis", symabi, "-o", obj, goSrc)
56+
cmd = testenv.Command(t, testenv.GoToolPath(t), "tool", "compile", "-importcfg="+importcfgfile, "-p=p", "-symabis", symabi, "-o", obj, goSrc)
5757
out, err = cmd.CombinedOutput()
5858
if err == nil {
5959
t.Fatalf("go tool compile did not fail")

src/internal/goroot/importcfg.go

-66
This file was deleted.

src/internal/testenv/testenv.go

+39-12
Original file line numberDiff line numberDiff line change
@@ -11,11 +11,11 @@
1111
package testenv
1212

1313
import (
14+
"bytes"
1415
"errors"
1516
"flag"
1617
"fmt"
1718
"internal/cfg"
18-
"internal/goroot"
1919
"internal/platform"
2020
"os"
2121
"os/exec"
@@ -347,18 +347,45 @@ func SkipIfOptimizationOff(t testing.TB) {
347347
}
348348

349349
// WriteImportcfg writes an importcfg file used by the compiler or linker to
350-
// dstPath containing entries for the packages in std and cmd in addition
351-
// to the package to package file mappings in additionalPackageFiles.
352-
func WriteImportcfg(t testing.TB, dstPath string, additionalPackageFiles map[string]string) {
353-
importcfg, err := goroot.Importcfg()
354-
for k, v := range additionalPackageFiles {
355-
importcfg += fmt.Sprintf("\npackagefile %s=%s", k, v)
350+
// dstPath containing entries for the file mappings in packageFiles, as well
351+
// as for the packages transitively imported by the package(s) in pkgs.
352+
//
353+
// pkgs may include any package pattern that is valid to pass to 'go list',
354+
// so it may also be a list of Go source files all in the same directory.
355+
func WriteImportcfg(t testing.TB, dstPath string, packageFiles map[string]string, pkgs ...string) {
356+
t.Helper()
357+
358+
icfg := new(bytes.Buffer)
359+
icfg.WriteString("# import config\n")
360+
for k, v := range packageFiles {
361+
fmt.Fprintf(icfg, "packagefile %s=%s\n", k, v)
356362
}
357-
if err != nil {
358-
t.Fatalf("preparing the importcfg failed: %s", err)
363+
364+
if len(pkgs) > 0 {
365+
// Use 'go list' to resolve any missing packages and rewrite the import map.
366+
cmd := Command(t, GoToolPath(t), "list", "-export", "-deps", "-f", `{{if ne .ImportPath "command-line-arguments"}}{{if .Export}}{{.ImportPath}}={{.Export}}{{end}}{{end}}`)
367+
cmd.Args = append(cmd.Args, pkgs...)
368+
cmd.Stderr = new(strings.Builder)
369+
out, err := cmd.Output()
370+
if err != nil {
371+
t.Fatalf("%v: %v\n%s", cmd, err, cmd.Stderr)
372+
}
373+
374+
for _, line := range strings.Split(string(out), "\n") {
375+
if line == "" {
376+
continue
377+
}
378+
importPath, export, ok := strings.Cut(line, "=")
379+
if !ok {
380+
t.Fatalf("invalid line in output from %v:\n%s", cmd, line)
381+
}
382+
if packageFiles[importPath] == "" {
383+
fmt.Fprintf(icfg, "packagefile %s=%s\n", importPath, export)
384+
}
385+
}
359386
}
360-
err = os.WriteFile(dstPath, []byte(importcfg), 0655)
361-
if err != nil {
362-
t.Fatalf("writing the importcfg failed: %s", err)
387+
388+
if err := os.WriteFile(dstPath, icfg.Bytes(), 0666); err != nil {
389+
t.Fatal(err)
363390
}
364391
}

0 commit comments

Comments
 (0)