Skip to content

Commit f540ee6

Browse files
Bryan C. Millsgopherbot
Bryan C. Mills
authored andcommitted
internal/gcimporter: load cached export data for packages individually
In short tests, also avoid creating export data for all of std. This change applies the same improvements made to the equivalent std packages in CL 454497 and CL 454498. (It may even fix the reverse builders that are currently timing out in x/tools, although I suspect there is still a bit of work to do for those.) For golang/go#56967. Updates golang/go#47257. Change-Id: I82e72557da5f917203637513122932c7942a98e9 Reviewed-on: https://go-review.googlesource.com/c/tools/+/454499 Auto-Submit: Bryan Mills <[email protected]> TryBot-Result: Gopher Robot <[email protected]> Reviewed-by: Michael Matloob <[email protected]> Run-TryBot: Bryan Mills <[email protected]> gopls-CI: kokoro <[email protected]>
1 parent d444fa3 commit f540ee6

File tree

3 files changed

+88
-30
lines changed

3 files changed

+88
-30
lines changed

internal/gcimporter/gcimporter.go

+43-21
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ package gcimporter // import "golang.org/x/tools/internal/gcimporter"
1313

1414
import (
1515
"bufio"
16+
"bytes"
1617
"errors"
1718
"fmt"
1819
"go/build"
@@ -22,14 +23,13 @@ import (
2223
"io"
2324
"io/ioutil"
2425
"os"
25-
"path"
26+
"os/exec"
2627
"path/filepath"
2728
"sort"
2829
"strconv"
2930
"strings"
31+
"sync"
3032
"text/scanner"
31-
32-
"golang.org/x/tools/internal/goroot"
3333
)
3434

3535
const (
@@ -41,23 +41,45 @@ const (
4141
trace = false
4242
)
4343

44-
func lookupGorootExport(pkgpath, srcRoot, srcDir string) (string, bool) {
45-
pkgpath = filepath.ToSlash(pkgpath)
46-
m, err := goroot.PkgfileMap()
47-
if err != nil {
48-
return "", false
49-
}
50-
if export, ok := m[pkgpath]; ok {
51-
return export, true
52-
}
53-
vendorPrefix := "vendor"
54-
if strings.HasPrefix(srcDir, filepath.Join(srcRoot, "cmd")) {
55-
vendorPrefix = path.Join("cmd", vendorPrefix)
44+
var exportMap sync.Map // package dir → func() (string, bool)
45+
46+
// lookupGorootExport returns the location of the export data
47+
// (normally found in the build cache, but located in GOROOT/pkg
48+
// in prior Go releases) for the package located in pkgDir.
49+
//
50+
// (We use the package's directory instead of its import path
51+
// mainly to simplify handling of the packages in src/vendor
52+
// and cmd/vendor.)
53+
func lookupGorootExport(pkgDir string) (string, bool) {
54+
f, ok := exportMap.Load(pkgDir)
55+
if !ok {
56+
var (
57+
listOnce sync.Once
58+
exportPath string
59+
)
60+
f, _ = exportMap.LoadOrStore(pkgDir, func() (string, bool) {
61+
listOnce.Do(func() {
62+
cmd := exec.Command("go", "list", "-export", "-f", "{{.Export}}", pkgDir)
63+
cmd.Dir = build.Default.GOROOT
64+
var output []byte
65+
output, err := cmd.Output()
66+
if err != nil {
67+
return
68+
}
69+
70+
exports := strings.Split(string(bytes.TrimSpace(output)), "\n")
71+
if len(exports) != 1 {
72+
return
73+
}
74+
75+
exportPath = exports[0]
76+
})
77+
78+
return exportPath, exportPath != ""
79+
})
5680
}
57-
pkgpath = path.Join(vendorPrefix, pkgpath)
58-
fmt.Fprintln(os.Stderr, "looking up ", pkgpath)
59-
export, ok := m[pkgpath]
60-
return export, ok
81+
82+
return f.(func() (string, bool))()
6183
}
6284

6385
var pkgExts = [...]string{".a", ".o"}
@@ -83,8 +105,8 @@ func FindPkg(path, srcDir string) (filename, id string) {
83105
bp, _ := build.Import(path, srcDir, build.FindOnly|build.AllowBinary)
84106
if bp.PkgObj == "" {
85107
var ok bool
86-
if bp.Goroot {
87-
filename, ok = lookupGorootExport(path, bp.SrcRoot, srcDir)
108+
if bp.Goroot && bp.Dir != "" {
109+
filename, ok = lookupGorootExport(bp.Dir)
88110
}
89111
if !ok {
90112
id = path // make sure we have an id to print in error message

internal/gcimporter/gcimporter_test.go

+41-5
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@ import (
2727
"testing"
2828
"time"
2929

30+
"golang.org/x/tools/internal/goroot"
3031
"golang.org/x/tools/internal/testenv"
3132
)
3233

@@ -65,8 +66,20 @@ func compilePkg(t *testing.T, dirname, filename, outdirname string, packagefiles
6566
}
6667
objname := basename + ".o"
6768
outname := filepath.Join(outdirname, objname)
68-
importcfgfile := filepath.Join(outdirname, basename) + ".importcfg"
69-
testenv.WriteImportcfg(t, importcfgfile, packagefiles)
69+
70+
importcfgfile := os.DevNull
71+
if len(packagefiles) > 0 {
72+
importcfgfile = filepath.Join(outdirname, basename) + ".importcfg"
73+
importcfg := new(bytes.Buffer)
74+
fmt.Fprintf(importcfg, "# import config")
75+
for k, v := range packagefiles {
76+
fmt.Fprintf(importcfg, "\npackagefile %s=%s\n", k, v)
77+
}
78+
if err := os.WriteFile(importcfgfile, importcfg.Bytes(), 0655); err != nil {
79+
t.Fatal(err)
80+
}
81+
}
82+
7083
importreldir := strings.ReplaceAll(outdirname, string(os.PathSeparator), "/")
7184
cmd := exec.Command("go", "tool", "compile", "-p", pkg, "-D", importreldir, "-importcfg", importcfgfile, "-o", outname, filename)
7285
cmd.Dir = dirname
@@ -109,7 +122,16 @@ func TestImportTestdata(t *testing.T) {
109122
tmpdir := mktmpdir(t)
110123
defer os.RemoveAll(tmpdir)
111124

112-
compile(t, "testdata", testfile, filepath.Join(tmpdir, "testdata"), nil)
125+
packageFiles := map[string]string{}
126+
for _, pkg := range []string{"go/ast", "go/token"} {
127+
export, _ := FindPkg(pkg, "testdata")
128+
if export == "" {
129+
t.Fatalf("no export data found for %s", pkg)
130+
}
131+
packageFiles[pkg] = export
132+
}
133+
134+
compile(t, "testdata", testfile, filepath.Join(tmpdir, "testdata"), packageFiles)
113135

114136
// filename should end with ".go"
115137
filename := testfile[:len(testfile)-3]
@@ -137,6 +159,10 @@ func TestImportTestdata(t *testing.T) {
137159
}
138160

139161
func TestImportTypeparamTests(t *testing.T) {
162+
if testing.Short() {
163+
t.Skipf("in short mode, skipping test that requires export data for all of std")
164+
}
165+
140166
testenv.NeedsGo1Point(t, 18) // requires generics
141167

142168
// This package only handles gc export data.
@@ -191,7 +217,11 @@ func TestImportTypeparamTests(t *testing.T) {
191217

192218
// Compile and import, and compare the resulting package with the package
193219
// that was type-checked directly.
194-
compile(t, rootDir, entry.Name(), filepath.Join(tmpdir, "testdata"), nil)
220+
pkgFiles, err := goroot.PkgfileMap()
221+
if err != nil {
222+
t.Fatal(err)
223+
}
224+
compile(t, rootDir, entry.Name(), filepath.Join(tmpdir, "testdata"), pkgFiles)
195225
pkgName := strings.TrimSuffix(entry.Name(), ".go")
196226
imported := importPkg(t, "./testdata/"+pkgName, tmpdir)
197227
checked := checkFile(t, filename, src)
@@ -589,7 +619,13 @@ func TestIssue13566(t *testing.T) {
589619
if err != nil {
590620
t.Fatal(err)
591621
}
592-
compilePkg(t, "testdata", "a.go", testoutdir, nil, apkg(testoutdir))
622+
623+
jsonExport, _ := FindPkg("encoding/json", "testdata")
624+
if jsonExport == "" {
625+
t.Fatalf("no export data found for encoding/json")
626+
}
627+
628+
compilePkg(t, "testdata", "a.go", testoutdir, map[string]string{"encoding/json": jsonExport}, apkg(testoutdir))
593629
compile(t, testoutdir, bpath, testoutdir, map[string]string{apkg(testoutdir): filepath.Join(testoutdir, "a.o")})
594630

595631
// import must succeed (test for issue at hand)

internal/goroot/importcfg.go

+4-4
Original file line numberDiff line numberDiff line change
@@ -29,9 +29,7 @@ func Importcfg() (string, error) {
2929
}
3030
fmt.Fprintf(&icfg, "# import config")
3131
for importPath, export := range m {
32-
if importPath != "unsafe" && export != "" { // unsafe
33-
fmt.Fprintf(&icfg, "\npackagefile %s=%s", importPath, export)
34-
}
32+
fmt.Fprintf(&icfg, "\npackagefile %s=%s", importPath, export)
3533
}
3634
s := icfg.String()
3735
return s, nil
@@ -63,7 +61,9 @@ func PkgfileMap() (map[string]string, error) {
6361
return
6462
}
6563
importPath, export := sp[0], sp[1]
66-
m[importPath] = export
64+
if export != "" {
65+
m[importPath] = export
66+
}
6767
}
6868
stdlibPkgfileMap = m
6969
})

0 commit comments

Comments
 (0)