From b1e4acd68ab67534bb0d0aea1080e6f27c9f3032 Mon Sep 17 00:00:00 2001 From: Ian Cottrell Date: Tue, 24 Jul 2018 14:54:35 -0400 Subject: [PATCH] go/packages: alter the internal api to go list This separates the go list specific behavior from the generalised go/packages loading behaviour, to enable alternate build system back ends. Change-Id: I07e7649f8f2afc7470a3ee3d0d743cbbcc6f713e Reviewed-on: https://go-review.googlesource.com/125715 Run-TryBot: Ian Cottrell TryBot-Result: Gobot Gobot Reviewed-by: Michael Matloob --- go/packages/golist.go | 24 ++++----- go/packages/loader_fallback.go | 13 +++-- go/packages/packages.go | 96 +++++++++++++++++----------------- go/packages/raw.go | 35 +++++++++++++ 4 files changed, 105 insertions(+), 63 deletions(-) diff --git a/go/packages/golist.go b/go/packages/golist.go index 1b4861ef43f..1793afbf562 100644 --- a/go/packages/golist.go +++ b/go/packages/golist.go @@ -30,7 +30,7 @@ type GoTooOldError struct { // golistPackages uses the "go list" command to expand the // pattern words and return metadata for the specified packages. // dir may be "" and env may be nil, as per os/exec.Command. -func golistPackages(cfg *rawConfig, words ...string) ([]*loaderPackage, error) { +func golistPackages(cfg *rawConfig, words ...string) ([]*rawPackage, error) { // Fields must match go list; // see $GOROOT/src/cmd/go/internal/load/pkg.go. type jsonPackage struct { @@ -71,7 +71,7 @@ func golistPackages(cfg *rawConfig, words ...string) ([]*loaderPackage, error) { return nil, err } // Decode the JSON and convert it to Package form. - var result []*loaderPackage + var result []*rawPackage for dec := json.NewDecoder(buf); dec.More(); { p := new(jsonPackage) if err := dec.Decode(p); err != nil { @@ -150,17 +150,15 @@ func golistPackages(cfg *rawConfig, words ...string) ([]*loaderPackage, error) { imports[id] = id // identity import } - pkg := &loaderPackage{ - Package: &Package{ - ID: id, - Name: p.Name, - GoFiles: absJoin(p.Dir, p.GoFiles, p.CgoFiles), - OtherFiles: absJoin(p.Dir, p.SFiles, p.CFiles), - }, - pkgpath: pkgpath, - imports: imports, - export: export, - indirect: p.DepOnly, + pkg := &rawPackage{ + ID: id, + Name: p.Name, + GoFiles: absJoin(p.Dir, p.GoFiles, p.CgoFiles), + OtherFiles: absJoin(p.Dir, p.SFiles, p.CFiles), + PkgPath: pkgpath, + Imports: imports, + Export: export, + DepOnly: p.DepOnly, } result = append(result, pkg) } diff --git a/go/packages/loader_fallback.go b/go/packages/loader_fallback.go index f92fe03839d..4a7384db26b 100644 --- a/go/packages/loader_fallback.go +++ b/go/packages/loader_fallback.go @@ -23,9 +23,10 @@ package packages import ( "fmt" "go/build" + "strings" + legacy "golang.org/x/tools/go/loader" "golang.org/x/tools/imports" - "strings" ) func loaderFallback(dir string, env []string, patterns []string) ([]*Package, error) { @@ -84,6 +85,13 @@ func loaderFallback(dir string, env []string, patterns []string) ([]*Package, er } allpkgs[id] = &loaderPackage{ + raw: &rawPackage{ + ID: id, + Name: lpkg.Pkg.Name(), + Imports: pkgimports, + GoFiles: goFiles, + DepOnly: !initial[lpkg], + }, Package: &Package{ ID: id, Name: lpkg.Pkg.Name(), @@ -96,14 +104,13 @@ func loaderFallback(dir string, env []string, patterns []string) ([]*Package, er IllTyped: !lpkg.TransitivelyErrorFree, OtherFiles: nil, // Never set for the fallback, because we can't extract from loader. }, - imports: pkgimports, } } // Do a second pass to populate imports. for _, pkg := range allpkgs { pkg.Imports = make(map[string]*Package) - for imppath, impid := range pkg.imports { + for imppath, impid := range pkg.raw.Imports { target, ok := allpkgs[impid] if !ok { // return nil, fmt.Errorf("could not load package: %v", impid) diff --git a/go/packages/packages.go b/go/packages/packages.go index c570acbdf9d..cd1201d54e9 100644 --- a/go/packages/packages.go +++ b/go/packages/packages.go @@ -135,10 +135,7 @@ type Config struct { // Load and returns the Go packages named by the given patterns. func Load(cfg *Config, patterns ...string) ([]*Package, error) { - l := &loader{} - if cfg != nil { - l.Config = *cfg - } + l := newLoader(cfg) return l.load(patterns...) } @@ -209,23 +206,12 @@ type Package struct { // loaderPackage augments Package with state used during the loading phase type loaderPackage struct { + raw *rawPackage *Package - - // export holds the path to the export data file - // for this package, if mode == TypeCheck. - // The export data file contains the package's type information - // in a compiler-specific format; see - // golang.org/x/tools/go/{gc,gccgo}exportdata. - // May be the empty string if the build failed. - export string - - indirect bool // package is a dependency, not explicitly requested - imports map[string]string // nominal form of Imports graph - importErrors map[string]error // maps each bad import to its error + importErrors map[string]error // maps each bad import to its error loadOnce sync.Once color uint8 // for cycle detection mark, needsrc bool // used in TypeCheck mode only - pkgpath string } func (lpkg *Package) String() string { return lpkg.ID } @@ -237,7 +223,11 @@ type loader struct { exportMu sync.Mutex // enforces mutual exclusion of exportdata operations } -func (ld *loader) load(patterns ...string) ([]*Package, error) { +func newLoader(cfg *Config) *loader { + ld := &loader{} + if cfg != nil { + ld.Config = *cfg + } if ld.Context == nil { ld.Context = context.Background() } @@ -259,7 +249,10 @@ func (ld *loader) load(patterns ...string) ([]*Package, error) { } } } + return ld +} +func (ld *loader) load(patterns ...string) ([]*Package, error) { if len(patterns) == 0 { return nil, fmt.Errorf("no packages to load") } @@ -274,27 +267,39 @@ func (ld *loader) load(patterns ...string) ([]*Package, error) { if err != nil { return nil, err } - ld.pkgs = make(map[string]*loaderPackage) + return ld.loadFrom(list...) +} + +func (ld *loader) loadFrom(list ...*rawPackage) ([]*Package, error) { + ld.pkgs = make(map[string]*loaderPackage, len(list)) var initial []*loaderPackage + // first pass, fixup and build the map and roots for _, pkg := range list { - ld.pkgs[pkg.ID] = pkg - - // Record the set of initial packages - // corresponding to the patterns. - if !pkg.indirect { - initial = append(initial, pkg) - - if ld.Mode == LoadSyntax { - pkg.needsrc = true - } + lpkg := &loaderPackage{ + raw: pkg, + Package: &Package{ + ID: pkg.ID, + Name: pkg.Name, + GoFiles: pkg.GoFiles, + OtherFiles: pkg.OtherFiles, + }, + // TODO: should needsrc also be true if pkg.Export == "" + needsrc: ld.Mode >= LoadAllSyntax, } - if ld.Mode >= LoadAllSyntax { - pkg.needsrc = true + ld.pkgs[lpkg.ID] = lpkg + if !pkg.DepOnly { + initial = append(initial, lpkg) + if ld.Mode == LoadSyntax { + lpkg.needsrc = true + } } } if len(ld.pkgs) == 0 { return nil, fmt.Errorf("packages not found") } + if len(initial) == 0 { + return nil, fmt.Errorf("packages had no initial set") + } // Materialize the import graph. @@ -325,9 +330,8 @@ func (ld *loader) load(patterns ...string) ([]*Package, error) { } lpkg.color = grey stack = append(stack, lpkg) // push - - imports := make(map[string]*Package) - for importPath, id := range lpkg.imports { + lpkg.Imports = make(map[string]*Package, len(lpkg.raw.Imports)) + for importPath, id := range lpkg.raw.Imports { var importErr error imp := ld.pkgs[id] if imp == nil { @@ -347,10 +351,8 @@ func (ld *loader) load(patterns ...string) ([]*Package, error) { if visit(imp) { lpkg.needsrc = true } - imports[importPath] = imp.Package + lpkg.Imports[importPath] = imp.Package } - lpkg.imports = nil // no longer needed - lpkg.Imports = imports stack = stack[:len(stack)-1] // pop lpkg.color = black @@ -414,7 +416,7 @@ func (ld *loader) loadRecursive(lpkg *loaderPackage) { // after immediate dependencies are loaded. // Precondition: ld.mode != Metadata. func (ld *loader) loadPackage(lpkg *loaderPackage) { - if lpkg.pkgpath == "unsafe" { + if lpkg.raw.PkgPath == "unsafe" { // Fill in the blanks to avoid surprises. lpkg.Types = types.Unsafe lpkg.Fset = ld.Fset @@ -449,7 +451,7 @@ func (ld *loader) loadPackage(lpkg *loaderPackage) { // Call NewPackage directly with explicit name. // This avoids skew between golist and go/types when the files' // package declarations are inconsistent. - lpkg.Types = types.NewPackage(lpkg.pkgpath, lpkg.Name) + lpkg.Types = types.NewPackage(lpkg.raw.PkgPath, lpkg.Name) lpkg.TypesInfo = &types.Info{ Types: make(map[ast.Expr]types.TypeAndValue), @@ -587,7 +589,7 @@ func (ld *loader) parseFiles(filenames []string) ([]*ast.File, []error) { // loadFromExportData returns type information for the specified // package, loading it from an export data file on the first request. func (ld *loader) loadFromExportData(lpkg *loaderPackage) (*types.Package, error) { - if lpkg.pkgpath == "" { + if lpkg.raw.PkgPath == "" { log.Fatalf("internal error: Package %s has no PkgPath", lpkg) } @@ -612,11 +614,11 @@ func (ld *loader) loadFromExportData(lpkg *loaderPackage) (*types.Package, error lpkg.IllTyped = true // fail safe - if lpkg.export == "" { + if lpkg.raw.Export == "" { // Errors while building export data will have been printed to stderr. return nil, fmt.Errorf("no export data file") } - f, err := os.Open(lpkg.export) + f, err := os.Open(lpkg.raw.Export) if err != nil { return nil, err } @@ -630,7 +632,7 @@ func (ld *loader) loadFromExportData(lpkg *loaderPackage) (*types.Package, error // queries.) r, err := gcexportdata.NewReader(f) if err != nil { - return nil, fmt.Errorf("reading %s: %v", lpkg.export, err) + return nil, fmt.Errorf("reading %s: %v", lpkg.raw.Export, err) } // Build the view. @@ -671,7 +673,7 @@ func (ld *loader) loadFromExportData(lpkg *loaderPackage) (*types.Package, error if !seen[p] { seen[p] = true if p.Types != nil { - view[p.pkgpath] = p.Types + view[p.raw.PkgPath] = p.Types } else { copyback = append(copyback, p) } @@ -684,15 +686,15 @@ func (ld *loader) loadFromExportData(lpkg *loaderPackage) (*types.Package, error // Parse the export data. // (May create/modify packages in view.) - tpkg, err := gcexportdata.Read(r, ld.Fset, view, lpkg.pkgpath) + tpkg, err := gcexportdata.Read(r, ld.Fset, view, lpkg.raw.PkgPath) if err != nil { - return nil, fmt.Errorf("reading %s: %v", lpkg.export, err) + return nil, fmt.Errorf("reading %s: %v", lpkg.raw.Export, err) } // For each newly created types.Package in the view, // save it in the main graph. for _, p := range copyback { - p.Types = view[p.pkgpath] // may still be nil + p.Types = view[p.raw.PkgPath] // may still be nil } lpkg.Types = tpkg diff --git a/go/packages/raw.go b/go/packages/raw.go index b237f2ea671..0affe3ffc40 100644 --- a/go/packages/raw.go +++ b/go/packages/raw.go @@ -13,6 +13,41 @@ import ( // This file contains the structs needed at the seam between the packages // loader and the underlying build tool +// rawPackage is the serialized form of a package +type rawPackage struct { + // ID is a unique identifier for a package. + // This is the same as Package.ID + ID string + // Name is the package name as it appears in the package source code. + // This is the same as Package.name + Name string `json:",omitempty"` + // This is the package path as used in the export data. + // This is used to map entries in the export data back to the package they + // come from. + // This is not currently exposed in Package. + PkgPath string `json:",omitempty"` + // Imports maps import paths appearing in the package's Go source files + // to corresponding package identifiers. + // This is similar to Package.Imports, but maps to the ID rather than the + // package itself. + Imports map[string]string `json:",omitempty"` + // Export is the absolute path to a file containing the export data for the + // package. + // This is not currently exposed in Package. + Export string `json:",omitempty"` + // GoFiles lists the absolute file paths of the package's Go source files. + // This is the same as Package.GoFiles + GoFiles []string `json:",omitempty"` + // OtherFiles lists the absolute file paths of the package's non-Go source + // files. + // This is the same as Package.OtherFiles + OtherFiles []string `json:",omitempty"` + // DepOnly marks a package that is in a list because it was a dependency. + // It is used to find the roots when constructing a graph from a package list. + // This is not exposed in Package. + DepOnly bool `json:",omitempty"` +} + // rawConfig specifies details about what raw package information is needed // and how the underlying build tool should load package data. type rawConfig struct {