From f540ee6b175a7971fd009ac8bc072f972cf9490a Mon Sep 17 00:00:00 2001 From: "Bryan C. Mills" Date: Thu, 1 Dec 2022 17:03:08 -0500 Subject: [PATCH] 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 TryBot-Result: Gopher Robot Reviewed-by: Michael Matloob Run-TryBot: Bryan Mills gopls-CI: kokoro --- internal/gcimporter/gcimporter.go | 64 +++++++++++++++++--------- internal/gcimporter/gcimporter_test.go | 46 ++++++++++++++++-- internal/goroot/importcfg.go | 8 ++-- 3 files changed, 88 insertions(+), 30 deletions(-) diff --git a/internal/gcimporter/gcimporter.go b/internal/gcimporter/gcimporter.go index f8369cdc52e..1faaa365260 100644 --- a/internal/gcimporter/gcimporter.go +++ b/internal/gcimporter/gcimporter.go @@ -13,6 +13,7 @@ package gcimporter // import "golang.org/x/tools/internal/gcimporter" import ( "bufio" + "bytes" "errors" "fmt" "go/build" @@ -22,14 +23,13 @@ import ( "io" "io/ioutil" "os" - "path" + "os/exec" "path/filepath" "sort" "strconv" "strings" + "sync" "text/scanner" - - "golang.org/x/tools/internal/goroot" ) const ( @@ -41,23 +41,45 @@ const ( trace = false ) -func lookupGorootExport(pkgpath, srcRoot, srcDir string) (string, bool) { - pkgpath = filepath.ToSlash(pkgpath) - m, err := goroot.PkgfileMap() - if err != nil { - return "", false - } - if export, ok := m[pkgpath]; ok { - return export, true - } - vendorPrefix := "vendor" - if strings.HasPrefix(srcDir, filepath.Join(srcRoot, "cmd")) { - vendorPrefix = path.Join("cmd", vendorPrefix) +var exportMap sync.Map // package dir → func() (string, bool) + +// lookupGorootExport returns the location of the export data +// (normally found in the build cache, but located in GOROOT/pkg +// in prior Go releases) for the package located in pkgDir. +// +// (We use the package's directory instead of its import path +// mainly to simplify handling of the packages in src/vendor +// and cmd/vendor.) +func lookupGorootExport(pkgDir string) (string, bool) { + f, ok := exportMap.Load(pkgDir) + if !ok { + var ( + listOnce sync.Once + exportPath string + ) + f, _ = exportMap.LoadOrStore(pkgDir, func() (string, bool) { + listOnce.Do(func() { + cmd := exec.Command("go", "list", "-export", "-f", "{{.Export}}", pkgDir) + cmd.Dir = build.Default.GOROOT + var output []byte + output, err := cmd.Output() + if err != nil { + return + } + + exports := strings.Split(string(bytes.TrimSpace(output)), "\n") + if len(exports) != 1 { + return + } + + exportPath = exports[0] + }) + + return exportPath, exportPath != "" + }) } - pkgpath = path.Join(vendorPrefix, pkgpath) - fmt.Fprintln(os.Stderr, "looking up ", pkgpath) - export, ok := m[pkgpath] - return export, ok + + return f.(func() (string, bool))() } var pkgExts = [...]string{".a", ".o"} @@ -83,8 +105,8 @@ func FindPkg(path, srcDir string) (filename, id string) { bp, _ := build.Import(path, srcDir, build.FindOnly|build.AllowBinary) if bp.PkgObj == "" { var ok bool - if bp.Goroot { - filename, ok = lookupGorootExport(path, bp.SrcRoot, srcDir) + if bp.Goroot && bp.Dir != "" { + filename, ok = lookupGorootExport(bp.Dir) } if !ok { id = path // make sure we have an id to print in error message diff --git a/internal/gcimporter/gcimporter_test.go b/internal/gcimporter/gcimporter_test.go index d04668e6312..1e1dd358453 100644 --- a/internal/gcimporter/gcimporter_test.go +++ b/internal/gcimporter/gcimporter_test.go @@ -27,6 +27,7 @@ import ( "testing" "time" + "golang.org/x/tools/internal/goroot" "golang.org/x/tools/internal/testenv" ) @@ -65,8 +66,20 @@ func compilePkg(t *testing.T, dirname, filename, outdirname string, packagefiles } objname := basename + ".o" outname := filepath.Join(outdirname, objname) - importcfgfile := filepath.Join(outdirname, basename) + ".importcfg" - testenv.WriteImportcfg(t, importcfgfile, packagefiles) + + importcfgfile := os.DevNull + if len(packagefiles) > 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) + } + } + importreldir := strings.ReplaceAll(outdirname, string(os.PathSeparator), "/") cmd := exec.Command("go", "tool", "compile", "-p", pkg, "-D", importreldir, "-importcfg", importcfgfile, "-o", outname, filename) cmd.Dir = dirname @@ -109,7 +122,16 @@ func TestImportTestdata(t *testing.T) { tmpdir := mktmpdir(t) defer os.RemoveAll(tmpdir) - compile(t, "testdata", testfile, filepath.Join(tmpdir, "testdata"), nil) + packageFiles := map[string]string{} + for _, pkg := range []string{"go/ast", "go/token"} { + 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) // filename should end with ".go" filename := testfile[:len(testfile)-3] @@ -137,6 +159,10 @@ func TestImportTestdata(t *testing.T) { } func TestImportTypeparamTests(t *testing.T) { + if testing.Short() { + t.Skipf("in short mode, skipping test that requires export data for all of std") + } + testenv.NeedsGo1Point(t, 18) // requires generics // This package only handles gc export data. @@ -191,7 +217,11 @@ func TestImportTypeparamTests(t *testing.T) { // Compile and import, and compare the resulting package with the package // that was type-checked directly. - compile(t, rootDir, entry.Name(), filepath.Join(tmpdir, "testdata"), nil) + pkgFiles, err := goroot.PkgfileMap() + if err != nil { + t.Fatal(err) + } + compile(t, rootDir, entry.Name(), filepath.Join(tmpdir, "testdata"), pkgFiles) pkgName := strings.TrimSuffix(entry.Name(), ".go") imported := importPkg(t, "./testdata/"+pkgName, tmpdir) checked := checkFile(t, filename, src) @@ -589,7 +619,13 @@ func TestIssue13566(t *testing.T) { if err != nil { t.Fatal(err) } - compilePkg(t, "testdata", "a.go", testoutdir, nil, apkg(testoutdir)) + + jsonExport, _ := FindPkg("encoding/json", "testdata") + if jsonExport == "" { + t.Fatalf("no export data found for encoding/json") + } + + compilePkg(t, "testdata", "a.go", testoutdir, map[string]string{"encoding/json": jsonExport}, apkg(testoutdir)) compile(t, testoutdir, bpath, testoutdir, map[string]string{apkg(testoutdir): filepath.Join(testoutdir, "a.o")}) // import must succeed (test for issue at hand) diff --git a/internal/goroot/importcfg.go b/internal/goroot/importcfg.go index 6575cfb9df6..f1cd28e2ec3 100644 --- a/internal/goroot/importcfg.go +++ b/internal/goroot/importcfg.go @@ -29,9 +29,7 @@ func Importcfg() (string, error) { } fmt.Fprintf(&icfg, "# import config") for importPath, export := range m { - if importPath != "unsafe" && export != "" { // unsafe - fmt.Fprintf(&icfg, "\npackagefile %s=%s", importPath, export) - } + fmt.Fprintf(&icfg, "\npackagefile %s=%s", importPath, export) } s := icfg.String() return s, nil @@ -63,7 +61,9 @@ func PkgfileMap() (map[string]string, error) { return } importPath, export := sp[0], sp[1] - m[importPath] = export + if export != "" { + m[importPath] = export + } } stdlibPkgfileMap = m })