[tools] go/internal/gcimporter: delete FindPkg and Import
43 views
Skip to first unread message
Russ Cox (Gerrit)
Apr 15, 2021, 12:59:02 PM4/15/21
to goph...@pubsubhelper.golang.org, Russ Cox, golang-co...@googlegroups.com
Russ Cox has uploaded this change for review.
go/internal/gcimporter: delete FindPkg and Import
These are unused (and this is an internal package)
and they also don't work in a module world, because
they poke around in GOPATH themselves rather than
invoke the go command.
Also fix up many tests to avoid using these routines.
This makes sure the tests in this package do not break
when we change the default GOEXPERIMENT set in the
main repo to have regabi on by default, which would set
build tags that this code is unaware of.
The CL looks big but the non-test changes are strictly
deletions.
Change-Id: Ia50940ca846fc03310b206106066674f574d2b4c
---
M go/internal/gcimporter/bexport_test.go
M go/internal/gcimporter/gcimporter.go
M go/internal/gcimporter/gcimporter_test.go
M go/internal/gcimporter/iexport_test.go
R go/internal/gcimporter/testdata/a/a.go
M go/internal/gcimporter/testdata/b.go
M go/internal/gcimporter/testdata/exports.go
A go/internal/gcimporter/testdata/haserrors/haserrors.go
8 files changed, 171 insertions(+), 395 deletions(-)
diff --git a/go/internal/gcimporter/bexport_test.go b/go/internal/gcimporter/bexport_test.go
index 702278e..fc0c291 100644
--- a/go/internal/gcimporter/bexport_test.go
+++ b/go/internal/gcimporter/bexport_test.go
@@ -7,7 +7,6 @@
import (
"fmt"
"go/ast"
- "go/build"
"go/constant"
"go/parser"
"go/token"
@@ -18,9 +17,8 @@
"strings"
"testing"
- "golang.org/x/tools/go/buildutil"
"golang.org/x/tools/go/internal/gcimporter"
- "golang.org/x/tools/go/loader"
+ "golang.org/x/tools/go/packages"
)
var isRace = false
@@ -36,43 +34,28 @@
t.Skipf("stdlib tests take too long in race mode and flake on builders")
}
- // Load, parse and type-check the program.
- ctxt := build.Default // copy
- ctxt.GOPATH = "" // disable GOPATH
- conf := loader.Config{
- Build: &ctxt,
- AllowErrors: true,
+ // Load, parse and type-check all of std as well as testdata/haserrors,
+ // which has type and value errors.
+ cfg := &packages.Config{
+ Mode: packages.NeedName | packages.NeedImports | packages.NeedTypes,
}
- for _, path := range buildutil.AllPackages(conf.Build) {
- conf.Import(path)
- }
-
- // Create a package containing type and value errors to ensure
- // they are properly encoded/decoded.
- f, err := conf.ParseFile("haserrors/haserrors.go", `package haserrors
-const UnknownValue = "" + 0
-type UnknownType undefined
-`)
+ pkgs, err := packages.Load(cfg, "./testdata/haserrors", "std")
if err != nil {
t.Fatal(err)
}
- conf.CreateFromFiles("haserrors", f)
- prog, err := conf.Load()
- if err != nil {
- t.Fatalf("Load failed: %v", err)
+ numPkgs := len(pkgs)
+ if want := 200; numPkgs < want {
+ t.Errorf("Loaded only %d packages, want at least %d\n%v", numPkgs, want, pkgs)
}
- numPkgs := len(prog.AllPackages)
- if want := 248; numPkgs < want {
- t.Errorf("Loaded only %d packages, want at least %d", numPkgs, want)
- }
-
- for pkg, info := range prog.AllPackages {
- if info.Files == nil {
- continue // empty directory
+ for _, packagesPkg := range pkgs {
+ pkg := packagesPkg.Types
+ if pkg.Path() == "unsafe" {
+ continue
}
- exportdata, err := gcimporter.BExportData(conf.Fset, pkg)
+ fset := packagesPkg.Fset
+ exportdata, err := gcimporter.BExportData(fset, pkg)
if err != nil {
t.Fatal(err)
}
@@ -101,7 +84,7 @@
continue
}
- fl1 := fileLine(conf.Fset, obj1)
+ fl1 := fileLine(fset, obj1)
fl2 := fileLine(fset2, obj2)
if fl1 != fl2 {
t.Errorf("%s.%s: got posn %s, want %s",
diff --git a/go/internal/gcimporter/gcimporter.go b/go/internal/gcimporter/gcimporter.go
index e8cba6b..e545708 100644
--- a/go/internal/gcimporter/gcimporter.go
+++ b/go/internal/gcimporter/gcimporter.go
@@ -12,17 +12,12 @@
package gcimporter // import "golang.org/x/tools/go/internal/gcimporter"
import (
- "bufio"
"errors"
"fmt"
- "go/build"
"go/constant"
"go/token"
"go/types"
"io"
- "io/ioutil"
- "os"
- "path/filepath"
"sort"
"strconv"
"strings"
@@ -32,66 +27,6 @@
// debugging/development support
const debug = false
-var pkgExts = [...]string{".a", ".o"}
-
-// FindPkg returns the filename and unique package id for an import
-// path based on package information provided by build.Import (using
-// the build.Default build.Context). A relative srcDir is interpreted
-// relative to the current working directory.
-// If no file was found, an empty filename is returned.
-//
-func FindPkg(path, srcDir string) (filename, id string) {
- if path == "" {
- return
- }
-
- var noext string
- switch {
- default:
- // "x" -> "$GOPATH/pkg/$GOOS_$GOARCH/x.ext", "x"
- // Don't require the source files to be present.
- if abs, err := filepath.Abs(srcDir); err == nil { // see issue 14282
- srcDir = abs
- }
- bp, _ := build.Import(path, srcDir, build.FindOnly|build.AllowBinary)
- if bp.PkgObj == "" {
- id = path // make sure we have an id to print in error message
- return
- }
- noext = strings.TrimSuffix(bp.PkgObj, ".a")
- id = bp.ImportPath
-
- case build.IsLocalImport(path):
- // "./x" -> "/this/directory/x.ext", "/this/directory/x"
- noext = filepath.Join(srcDir, path)
- id = noext
-
- case filepath.IsAbs(path):
- // for completeness only - go/build.Import
- // does not support absolute imports
- // "/x" -> "/x.ext", "/x"
- noext = path
- id = path
- }
-
- if false { // for debugging
- if path != id {
- fmt.Printf("%s -> %s\n", path, id)
- }
- }
-
- // try extensions
- for _, ext := range pkgExts {
- filename = noext + ext
- if f, err := os.Stat(filename); err == nil && !f.IsDir() {
- return
- }
- }
-
- filename = "" // not found
- return
-}
-
// ImportData imports a package by reading the gc-generated export data,
// adds the corresponding package object to the packages map indexed by id,
// and returns the object.
@@ -124,102 +59,6 @@
return
}
-// Import imports a gc-generated package given its import path and srcDir, adds
-// the corresponding package object to the packages map, and returns the object.
-// The packages map must contain all packages already imported.
-//
-func Import(packages map[string]*types.Package, path, srcDir string, lookup func(path string) (io.ReadCloser, error)) (pkg *types.Package, err error) {
- var rc io.ReadCloser
- var filename, id string
- if lookup != nil {
- // With custom lookup specified, assume that caller has
- // converted path to a canonical import path for use in the map.
- if path == "unsafe" {
- return types.Unsafe, nil
- }
- id = path
-
- // No need to re-import if the package was imported completely before.
- if pkg = packages[id]; pkg != nil && pkg.Complete() {
- return
- }
- f, err := lookup(path)
- if err != nil {
- return nil, err
- }
- rc = f
- } else {
- filename, id = FindPkg(path, srcDir)
- if filename == "" {
- if path == "unsafe" {
- return types.Unsafe, nil
- }
- return nil, fmt.Errorf("can't find import: %q", id)
- }
-
- // no need to re-import if the package was imported completely before
- if pkg = packages[id]; pkg != nil && pkg.Complete() {
- return
- }
-
- // open file
- f, err := os.Open(filename)
- if err != nil {
- return nil, err
- }
- defer func() {
- if err != nil {
- // add file name to error
- err = fmt.Errorf("%s: %v", filename, err)
- }
- }()
- rc = f
- }
- defer rc.Close()
-
- var hdr string
- buf := bufio.NewReader(rc)
- if hdr, err = FindExportData(buf); err != nil {
- return
- }
-
- switch hdr {
- case "$$\n":
- // Work-around if we don't have a filename; happens only if lookup != nil.
- // Either way, the filename is only needed for importer error messages, so
- // this is fine.
- if filename == "" {
- filename = path
- }
- return ImportData(packages, filename, id, buf)
-
- case "$$B\n":
- var data []byte
- data, err = ioutil.ReadAll(buf)
- if err != nil {
- break
- }
-
- // TODO(gri): allow clients of go/importer to provide a FileSet.
- // Or, define a new standard go/types/gcexportdata package.
- fset := token.NewFileSet()
-
- // The indexed export format starts with an 'i'; the older
- // binary export format starts with a 'c', 'd', or 'v'
- // (from "version"). Select appropriate importer.
- if len(data) > 0 && data[0] == 'i' {
- _, pkg, err = IImportData(fset, packages, data[1:], id)
- } else {
- _, pkg, err = BImportData(fset, packages, data, id)
- }
-
- default:
- err = fmt.Errorf("unknown export data header: %q", hdr)
- }
-
- return
-}
-
// ----------------------------------------------------------------------------
// Parser
diff --git a/go/internal/gcimporter/gcimporter_test.go b/go/internal/gcimporter/gcimporter_test.go
index cfafbbf..84e69ce 100644
--- a/go/internal/gcimporter/gcimporter_test.go
+++ b/go/internal/gcimporter/gcimporter_test.go
@@ -8,9 +8,12 @@
package gcimporter
import (
+ "bufio"
"bytes"
+ "encoding/json"
"fmt"
"go/constant"
+ "go/token"
"go/types"
"io/ioutil"
"os"
@@ -82,34 +85,13 @@
}
}
-// compile runs the compiler on filename, with dirname as the working directory,
-// and writes the output file to outdirname.
-func compile(t *testing.T, dirname, filename, outdirname string) string {
- /* testenv. */ MustHaveGoBuild(t)
- // filename must end with ".go"
- if !strings.HasSuffix(filename, ".go") {
- t.Fatalf("filename doesn't end in .go: %s", filename)
- }
- basename := filepath.Base(filename)
- outname := filepath.Join(outdirname, basename[:len(basename)-2]+"o")
- cmd := exec.Command("go", "tool", "compile", "-o", outname, filename)
- cmd.Dir = dirname
- out, err := cmd.CombinedOutput()
- if err != nil {
- t.Logf("%s", out)
- t.Fatalf("go tool compile %s failed: %s", filename, err)
- }
- return outname
-}
-
-func testPath(t *testing.T, path, srcDir string) *types.Package {
+func testPath(t *testing.T, file string) *types.Package {
t0 := time.Now()
- pkg, err := Import(make(map[string]*types.Package), path, srcDir, nil)
+ pkg, err := importFile("fake/import/path", file)
if err != nil {
- t.Errorf("testPath(%s): %s", path, err)
- return nil
+ t.Fatal(err)
}
- t.Logf("testPath(%s): %v", path, time.Since(t0))
+ t.Logf("testPath(%s): %v", file, time.Since(t0))
return pkg
}
@@ -129,12 +111,9 @@
switch {
case !f.IsDir():
// try extensions
- for _, ext := range pkgExts {
- if strings.HasSuffix(f.Name(), ext) {
- name := f.Name()[0 : len(f.Name())-len(ext)] // remove extension
- if testPath(t, filepath.Join(dir, name), dir) != nil {
- nimports++
- }
+ if strings.HasSuffix(f.Name(), ".a") {
+ if testPath(t, filepath.Join(dirname, f.Name())) != nil {
+ nimports++
}
}
case f.IsDir():
@@ -156,36 +135,23 @@
return tmpdir
}
-const testfile = "exports.go"
-
func TestImportTestdata(t *testing.T) {
// This package only handles gc export data.
if runtime.Compiler != "gc" {
t.Skipf("gc-built packages not available (compiler = %s)", runtime.Compiler)
}
- tmpdir := mktmpdir(t)
- defer os.RemoveAll(tmpdir)
+ pkg := importPkg(t, "./testdata/exports.go")
- compile(t, "testdata", testfile, filepath.Join(tmpdir, "testdata"))
-
- // filename should end with ".go"
- filename := testfile[:len(testfile)-3]
- if pkg := testPath(t, "./testdata/"+filename, tmpdir); pkg != nil {
- // The package's Imports list must include all packages
- // explicitly imported by testfile, plus all packages
- // referenced indirectly via exported objects in testfile.
- // With the textual export format (when run against Go1.6),
- // the list may also include additional packages that are
- // not strictly required for import processing alone (they
- // are exported to err "on the safe side").
- // For now, we just test the presence of a few packages
- // that we know are there for sure.
- got := fmt.Sprint(pkg.Imports())
- for _, want := range []string{"go/ast", "go/token"} {
- if !strings.Contains(got, want) {
- t.Errorf(`Package("exports").Imports() = %s, does not contain %s`, got, want)
- }
+ // The package's Imports list must include all packages
+ // explicitly imported by exports.go, plus all packages
+ // referenced indirectly via exported objects.
+ // For now, we just test the presence of a few packages
+ // that we know are there for sure.
+ got := fmt.Sprint(pkg.Imports())
+ for _, want := range []string{"go/ast", "go/token"} {
+ if !strings.Contains(got, want) {
+ t.Errorf(`Package("exports").Imports() = %s, does not contain %s`, got, want)
}
}
}
@@ -226,7 +192,7 @@
}
// test that export data can be imported
- _, err := Import(make(map[string]*types.Package), pkgpath, dir, nil)
+ _, err := importFile(pkgpath, filepath.Join(dir, name))
if err != nil {
// ok to fail if it fails with a newer version error for select files
if strings.Contains(err.Error(), "newer version") {
@@ -262,7 +228,7 @@
ioutil.WriteFile(filename, data, 0666)
// test that importing the corrupted file results in an error
- _, err = Import(make(map[string]*types.Package), pkgpath, corruptdir, nil)
+ _, err = importFile(pkgpath, filename)
if err == nil {
t.Errorf("import corrupted %q succeeded", pkgpath)
} else if msg := err.Error(); !strings.Contains(msg, "version skew") {
@@ -370,11 +336,7 @@
importPath := s[0]
objName := s[1]
- pkg, err := Import(make(map[string]*types.Package), importPath, ".", nil)
- if err != nil {
- t.Error(err)
- return nil
- }
+ pkg := importPkg(t, importPath)
obj := pkg.Scope().Lookup(objName)
if obj == nil {
@@ -428,7 +390,7 @@
t.Skipf("gc-built packages not available (compiler = %s)", runtime.Compiler)
}
- pkg := importPkg(t, "strings", ".")
+ pkg := importPkg(t, "strings")
scope := pkg.Scope()
for _, name := range scope.Names() {
@@ -457,13 +419,18 @@
t.Skipf("gc-built packages not available (compiler = %s)", runtime.Compiler)
}
- imports := make(map[string]*types.Package)
- _, err := Import(imports, "net/http", ".", nil)
- if err != nil {
- t.Fatal(err)
+ httpPkg := importPkg(t, "net/http")
+ var syncPkg *types.Package
+ for _, pkg := range httpPkg.Imports() {
+ if pkg.Path() == "sync" {
+ syncPkg = pkg
+ }
+ }
+ if syncPkg == nil {
+ t.Fatal("cannot find sync pkg inside net/http export data")
}
- mutex := imports["sync"].Scope().Lookup("Mutex").(*types.TypeName).Type()
+ mutex := syncPkg.Scope().Lookup("Mutex").(*types.TypeName).Type()
mset := types.NewMethodSet(types.NewPointer(mutex)) // methods of *sync.Mutex
sel := mset.Lookup(nil, "Lock")
lock := sel.Obj().(*types.Func)
@@ -486,22 +453,8 @@
t.Skip("avoid dealing with relative paths/drive letters on windows")
}
- tmpdir := mktmpdir(t)
- defer os.RemoveAll(tmpdir)
- testoutdir := filepath.Join(tmpdir, "testdata")
-
- // b.go needs to be compiled from the output directory so that the compiler can
- // find the compiled package a. We pass the full path to compile() so that we
- // don't have to copy the file to that directory.
- bpath, err := filepath.Abs(filepath.Join("testdata", "b.go"))
- if err != nil {
- t.Fatal(err)
- }
- compile(t, "testdata", "a.go", testoutdir)
- compile(t, testoutdir, bpath, testoutdir)
-
// import must succeed (test for issue at hand)
- pkg := importPkg(t, "./testdata/b", tmpdir)
+ pkg := importPkg(t, "./testdata/b.go")
// make sure all indirectly imported packages have names
for _, imp := range pkg.Imports() {
@@ -520,16 +473,12 @@
}
// import go/internal/gcimporter which imports go/types partially
- imports := make(map[string]*types.Package)
- _, err := Import(imports, "go/internal/gcimporter", ".", nil)
- if err != nil {
- t.Fatal(err)
- }
+ gcimporterPkg := importPkg(t, "go/internal/gcimporter")
// look for go/types package
var goTypesPkg *types.Package
- for path, pkg := range imports {
- if path == "go/types" {
+ for _, pkg := range gcimporterPkg.Imports() {
+ if pkg.Path() == "go/types" {
goTypesPkg = pkg
break
}
@@ -557,45 +506,6 @@
}
}
-func TestIssue15517(t *testing.T) {
- skipSpecialPlatforms(t)
-
- // This package only handles gc export data.
- if runtime.Compiler != "gc" {
- t.Skipf("gc-built packages not available (compiler = %s)", runtime.Compiler)
- }
-
- // On windows, we have to set the -D option for the compiler to avoid having a drive
- // letter and an illegal ':' in the import path - just skip it (see also issue #3483).
- if runtime.GOOS == "windows" {
- t.Skip("avoid dealing with relative paths/drive letters on windows")
- }
-
- tmpdir := mktmpdir(t)
- defer os.RemoveAll(tmpdir)
-
- compile(t, "testdata", "p.go", filepath.Join(tmpdir, "testdata"))
-
- // Multiple imports of p must succeed without redeclaration errors.
- // We use an import path that's not cleaned up so that the eventual
- // file path for the package is different from the package path; this
- // will expose the error if it is present.
- //
- // (Issue: Both the textual and the binary importer used the file path
- // of the package to be imported as key into the shared packages map.
- // However, the binary importer then used the package path to identify
- // the imported package to mark it as complete; effectively marking the
- // wrong package as complete. By using an "unclean" package path, the
- // file and package path are different, exposing the problem if present.
- // The same issue occurs with vendoring.)
- imports := make(map[string]*types.Package)
- for i := 0; i < 3; i++ {
- if _, err := Import(imports, "./././testdata/p", tmpdir, nil); err != nil {
- t.Fatal(err)
- }
- }
-}
-
func TestIssue15920(t *testing.T) {
skipSpecialPlatforms(t)
@@ -610,7 +520,7 @@
t.Skip("avoid dealing with relative paths/drive letters on windows")
}
- compileAndImportPkg(t, "issue15920")
+ importPkg(t, "./testdata/issue15920.go")
}
func TestIssue20046(t *testing.T) {
@@ -628,7 +538,7 @@
}
// "./issue20046".V.M must exist
- pkg := compileAndImportPkg(t, "issue20046")
+ pkg := importPkg(t, "./testdata/issue20046.go")
obj := lookupObj(t, pkg.Scope(), "V")
if m, index, indirect := types.LookupFieldOrMethod(obj.Type(), false, nil, "M"); m == nil {
t.Fatalf("V.M not found (index = %v, indirect = %v)", index, indirect)
@@ -650,24 +560,69 @@
t.Skip("avoid dealing with relative paths/drive letters on windows")
}
- compileAndImportPkg(t, "issue25301")
+ importPkg(t, "./testdata/issue25301.go")
}
-func importPkg(t *testing.T, path, srcDir string) *types.Package {
- pkg, err := Import(make(map[string]*types.Package), path, srcDir, nil)
+func findPkg(t *testing.T, name string) (importPath, exportFile string) {
+ out, err := exec.Command("go", "list", "-json", "-export", "--", name).CombinedOutput()
+ if err != nil {
+ t.Fatalf("go list %s: %v\n%s", name, err, out)
+ }
+ var data struct {
+ ImportPath string
+ Export string
+ }
+ json.Unmarshal(out, &data)
+ return data.ImportPath, data.Export
+}
+
+func importFile(importPath, exportFile string) (*types.Package, error) {
+ f, err := os.Open(exportFile)
+ if err != nil {
+ return nil, err
+ }
+ buf := bufio.NewReader(f)
+ hdr, err := FindExportData(buf)
+ if err != nil {
+ return nil, err
+ }
+ if hdr != "$$B\n" {
+ return nil, fmt.Errorf("unknown export data header: %q", hdr)
+ }
+
+ data, err := ioutil.ReadAll(buf)
+ if err != nil {
+ return nil, err
+ }
+
+ fset := token.NewFileSet()
+ imports := make(map[string]*types.Package)
+
+ if len(data) == 0 || data[0] != 'i' {
+ // Older binary export format.
+ _, pkg, err := BImportData(fset, imports, data, importPath)
+ if err != nil {
+ return nil, err
+ }
+ return pkg, nil
+ }
+
+ // Indexed binary export format.
+ _, pkg, err := IImportData(fset, imports, data[1:], importPath)
+ if err != nil {
+ return nil, err
+ }
+ return pkg, nil
+}
+
+func importPkg(t *testing.T, path string) *types.Package {
+ pkg, err := importFile(findPkg(t, path))
if err != nil {
t.Fatal(err)
}
return pkg
}
-func compileAndImportPkg(t *testing.T, name string) *types.Package {
- tmpdir := mktmpdir(t)
- defer os.RemoveAll(tmpdir)
- compile(t, "testdata", name+".go", filepath.Join(tmpdir, "testdata"))
- return importPkg(t, "./testdata/"+name, tmpdir)
-}
-
func lookupObj(t *testing.T, scope *types.Scope, name string) types.Object {
if obj := scope.Lookup(name); obj != nil {
return obj
diff --git a/go/internal/gcimporter/iexport_test.go b/go/internal/gcimporter/iexport_test.go
index 5385011..b7d42e4 100644
--- a/go/internal/gcimporter/iexport_test.go
+++ b/go/internal/gcimporter/iexport_test.go
@@ -14,7 +14,6 @@
"bytes"
"fmt"
"go/ast"
- "go/build"
"go/constant"
"go/parser"
"go/token"
@@ -28,9 +27,8 @@
"strings"
"testing"
- "golang.org/x/tools/go/buildutil"
"golang.org/x/tools/go/internal/gcimporter"
- "golang.org/x/tools/go/loader"
+ "golang.org/x/tools/go/packages"
)
func readExportFile(filename string) ([]byte, error) {
@@ -73,71 +71,69 @@
t.Skipf("stdlib tests take too long in race mode and flake on builders")
}
- // Load, parse and type-check the program.
- ctxt := build.Default // copy
- ctxt.GOPATH = "" // disable GOPATH
- conf := loader.Config{
- Build: &ctxt,
- AllowErrors: true,
- TypeChecker: types.Config{
- Sizes: types.SizesFor(ctxt.Compiler, ctxt.GOARCH),
- },
+ // Load, parse and type-check all of std as well as testdata/haserrors,
+ // which has type and value errors.
+ fset := token.NewFileSet()
+ cfg := &packages.Config{
+ Mode: packages.NeedName | packages.NeedExportFile | packages.NeedTypes,
+ Fset: fset,
}
- for _, path := range buildutil.AllPackages(conf.Build) {
- conf.Import(path)
- }
-
- // Create a package containing type and value errors to ensure
- // they are properly encoded/decoded.
- f, err := conf.ParseFile("haserrors/haserrors.go", `package haserrors
-const UnknownValue = "" + 0
-type UnknownType undefined
-`)
+ pkgs, err := packages.Load(cfg, "./testdata/haserrors", "std")
if err != nil {
t.Fatal(err)
}
- conf.CreateFromFiles("haserrors", f)
- prog, err := conf.Load()
- if err != nil {
- t.Fatalf("Load failed: %v", err)
+ numPkgs := len(pkgs)
+ if want := 200; numPkgs < want {
+ t.Errorf("Loaded only %d packages, want at least %d\n%v", numPkgs, want, pkgs)
}
- numPkgs := len(prog.AllPackages)
- if want := 248; numPkgs < want {
- t.Errorf("Loaded only %d packages, want at least %d", numPkgs, want)
- }
-
- var sorted []*types.Package
- for pkg, info := range prog.AllPackages {
- if info.Files != nil { // non-empty directory
- sorted = append(sorted, pkg)
- }
- }
- sort.Slice(sorted, func(i, j int) bool {
- return sorted[i].Path() < sorted[j].Path()
+ sort.Slice(pkgs, func(i, j int) bool {
+ return pkgs[i].Types.Path() < pkgs[j].Types.Path()
})
- for _, pkg := range sorted {
- if exportdata, err := iexport(conf.Fset, pkg); err != nil {
+ for _, packagesPkg := range pkgs {
+ pkg := packagesPkg.Types
+ if pkg.Path() == "unsafe" {
+ continue
+ }
+ if exportdata, err := iexport(fset, pkg); err != nil {
t.Error(err)
} else {
- testPkgData(t, conf.Fset, pkg, exportdata)
+ testPkgData(t, fset, pkg, exportdata)
}
- if pkg.Name() == "main" || pkg.Name() == "haserrors" {
- // skip; no export data
- } else if bp, err := ctxt.Import(pkg.Path(), "", build.FindOnly); err != nil {
- t.Log("warning:", err)
- } else if exportdata, err := readExportFile(bp.PkgObj); err != nil {
- t.Log("warning:", err)
- } else {
- testPkgData(t, conf.Fset, pkg, exportdata)
+ xfile := packagesPkg.ExportFile
+ if xfile != "" {
+ if exportdata, err := readExportFile(xfile); err != nil {
+ t.Log("warning:", err)
+ } else {
+ testPkgData(t, fset, pkg, exportdata)
+ }
+ }
+ }
+
+ have := make(map[*types.Package]bool)
+ for _, pkg := range pkgs {
+ have[pkg.Types] = true
+ }
+ for _, pkg := range pkgs {
+ for _, imp := range pkg.Types.Imports() {
+ if !have[imp] {
+ t.Fatalf("do not have %s -> %s", pkg.Types.Path(), imp.Path())
+ }
}
}
var bundle bytes.Buffer
- if err := gcimporter.IExportBundle(&bundle, conf.Fset, sorted); err != nil {
+ var typesPkgs []*types.Package
+ for _, pkg := range pkgs {
+ if pkg.Types.Path() == "unsafe" {
+ continue
+ }
+ typesPkgs = append(typesPkgs, pkg.Types)
+ }
+ if err := gcimporter.IExportBundle(&bundle, fset, typesPkgs); err != nil {
t.Fatal(err)
}
fset2 := token.NewFileSet()
@@ -147,8 +143,8 @@
t.Fatal(err)
}
- for i, pkg := range sorted {
- testPkg(t, conf.Fset, pkg, fset2, pkgs2[i])
+ for i, pkg := range typesPkgs {
+ testPkg(t, fset, pkg, fset2, pkgs2[i])
}
}
diff --git a/go/internal/gcimporter/testdata/a.go b/go/internal/gcimporter/testdata/a/a.go
similarity index 100%
rename from go/internal/gcimporter/testdata/a.go
rename to go/internal/gcimporter/testdata/a/a.go
diff --git a/go/internal/gcimporter/testdata/b.go b/go/internal/gcimporter/testdata/b.go
index 4196678..cda5a1b 100644
--- a/go/internal/gcimporter/testdata/b.go
+++ b/go/internal/gcimporter/testdata/b.go
@@ -6,6 +6,6 @@
package b
-import "./a"
+import "golang.org/x/tools/go/internal/gcimporter/testdata/a"
type A a.A
diff --git a/go/internal/gcimporter/testdata/exports.go b/go/internal/gcimporter/testdata/exports.go
index 8ee28b0..aa53f39 100644
--- a/go/internal/gcimporter/testdata/exports.go
+++ b/go/internal/gcimporter/testdata/exports.go
@@ -21,7 +21,7 @@
C1 = 3.14159265
C2 = 2.718281828i
C3 = -123.456e-789
- C4 = +123.456E+789
+ C4 = +123.456e+789
C5 = 1234i
C6 = "foo\n"
C7 = `bar\n`
@@ -80,10 +80,10 @@
V1 = -991.0
)
-func F1() {}
-func F2(x int) {}
-func F3() int { return 0 }
-func F4() float32 { return 0 }
-func F5(a, b, c int, u, v, w struct{ x, y T1 }, more ...interface{}) (p, q, r chan<- T10)
+func F1() {}
+func F2(x int) {}
+func F3() int { return 0 }
+func F4() float32 { return 0 }
+func F5(a, b, c int, u, v, w struct{ x, y T1 }, more ...interface{}) (p, q, r chan<- T10) { panic(1) }
-func (p *T1) M1()
+func (p *T1) M1() { panic(1) }
diff --git a/go/internal/gcimporter/testdata/haserrors/haserrors.go b/go/internal/gcimporter/testdata/haserrors/haserrors.go
new file mode 100644
index 0000000..4721857
--- /dev/null
+++ b/go/internal/gcimporter/testdata/haserrors/haserrors.go
@@ -0,0 +1,3 @@
+package haserrors
+const UnknownValue = "" + 0
+type UnknownType undefined
To view, visit change 310516. To unsubscribe, or for help writing mail filters, visit settings.
kokoro (Gerrit)
Apr 15, 2021, 1:08:13 PM4/15/21
to Russ Cox, goph...@pubsubhelper.golang.org, Go Bot, golang-co...@googlegroups.com
Kokoro presubmit build finished with status: SUCCESS
Logs at: https://source.cloud.google.com/results/invocations/5e597354-7249-4077-8f7a-55e13ecbd01b
Patch set 1:gopls-CI +1
Austin Clements (Gerrit)
Apr 15, 2021, 3:44:07 PM4/15/21
to Russ Cox, goph...@pubsubhelper.golang.org, Austin Clements, Go Bot, kokoro, golang-co...@googlegroups.com
Attention is currently required from: Russ Cox.
Patch set 1:Code-Review +2
2 comments:
Patchset:
I'm not sure I'm totally qualified to review this, but it also seems low-risk.
File go/internal/gcimporter/gcimporter_test.go:
if len(data) == 0 || data[0] != 'i' {
// Older binary export format.
_, pkg, err := BImportData(fset, imports, data, importPath)
if err != nil {
return nil, err
}
return pkg, nil
}
// Indexed binary export format.
_, pkg, err := IImportData(fset, imports, data[1:], importPath)
if err != nil {
return nil, err
}I wasn't really expecting to find this in a _test file.
To view, visit change 310516. To unsubscribe, or for help writing mail filters, visit settings.
Russ Cox (Gerrit)
Apr 16, 2021, 12:11:13 AM4/16/21
to Russ Cox, goph...@pubsubhelper.golang.org, Austin Clements, Go Bot, kokoro, golang-co...@googlegroups.com
1 comment:
File go/internal/gcimporter/gcimporter_test.go:
if len(data) == 0 || data[0] != 'i' {
// Older binary export format.
_, pkg, err := BImportData(fset, imports, data, importPath)
if err != nil {
return nil, err
}
return pkg, nil
}
// Indexed binary export format.
_, pkg, err := IImportData(fset, imports, data[1:], importPath)
if err != nil {
return nil, err
}I wasn't really expecting to find this in a _test file.
Part of me would rather delete the test but this keeps the test working.
In practice the real version of this code is supplied by go/gcexportdata
but that can't be imported here.
To view, visit change 310516. To unsubscribe, or for help writing mail filters, visit settings.
kokoro (Gerrit)
Mar 25, 2022, 5:17:21 PM3/25/22
to Russ Cox, goph...@pubsubhelper.golang.org, Austin Clements, Gopher Robot, golang-co...@googlegroups.com
Kokoro presubmit build finished with status: SUCCESS
Logs at: https://source.cloud.google.com/results/invocations/6de67b9f-f8d8-498a-bdb1-81c40b686b92
Patch set 2:gopls-CI +1
Reply all
Reply to author
Forward
0 new messages