From 7ebc57ba1992bada70db16c8d92e3444a97cbe07 Mon Sep 17 00:00:00 2001 From: Ian Cottrell Date: Tue, 31 Jul 2018 17:06:16 -0400 Subject: [PATCH] go/packages: remove DepOnly from the raw package All the other members of raw package are stable for any given package, DepOnly relates to the query patterns, not thepackages. Instead the raw functions now return the set of roots matched Other minor changes included: rawConfig.ExtraFlags -> rawConfig.Flags delete rawConfig.Context Change-Id: I3a729fa4557043f65da386eed94ab46fdfacb8ad Reviewed-on: https://go-review.googlesource.com/127035 Reviewed-by: Michael Matloob --- go/packages/golist.go | 29 +++++++++++++++-------- go/packages/golist_fallback.go | 41 +++++++++++++++++++------------- go/packages/packages.go | 43 +++++++++++++++++++--------------- go/packages/raw.go | 38 ++++++++++-------------------- 4 files changed, 80 insertions(+), 71 deletions(-) diff --git a/go/packages/golist.go b/go/packages/golist.go index 99f26622..ff3d66bf 100644 --- a/go/packages/golist.go +++ b/go/packages/golist.go @@ -8,6 +8,7 @@ package packages import ( "bytes" + "context" "encoding/json" "fmt" "log" @@ -52,7 +53,7 @@ type jsonPackage 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) ([]*rawPackage, error) { +func golistPackages(ctx context.Context, cfg *rawConfig, words ...string) ([]string, []*rawPackage, error) { // go list uses the following identifiers in ImportPath and Imports: // // "p" -- importable package or main (command) @@ -67,16 +68,17 @@ func golistPackages(cfg *rawConfig, words ...string) ([]*rawPackage, error) { // Run "go list" for complete // information on the specified packages. - buf, err := golist(cfg, golistargs(cfg, words)) + buf, err := golist(ctx, cfg, golistargs(cfg, words)) if err != nil { - return nil, err + return nil, nil, err } // Decode the JSON and convert it to rawPackage form. + var roots []string var result []*rawPackage for dec := json.NewDecoder(buf); dec.More(); { p := new(jsonPackage) if err := dec.Decode(p); err != nil { - return nil, fmt.Errorf("JSON decoding failed: %v", err) + return nil, nil, fmt.Errorf("JSON decoding failed: %v", err) } // Bad package? @@ -159,12 +161,14 @@ func golistPackages(cfg *rawConfig, words ...string) ([]*rawPackage, error) { PkgPath: pkgpath, Imports: imports, Export: export, - DepOnly: p.DepOnly, + } + if !p.DepOnly { + roots = append(roots, pkg.ID) } result = append(result, pkg) } - return result, nil + return roots, result, nil } // absJoin absolutizes and flattens the lists of files. @@ -181,17 +185,22 @@ func absJoin(dir string, fileses ...[]string) (res []string) { } func golistargs(cfg *rawConfig, words []string) []string { - fullargs := []string{"list", "-e", "-json", "-cgo=true"} - fullargs = append(fullargs, cfg.Flags()...) + fullargs := []string{ + "list", "-e", "-json", "-cgo=true", + fmt.Sprintf("-test=%t", cfg.Tests), + fmt.Sprintf("-export=%t", cfg.Export), + fmt.Sprintf("-deps=%t", cfg.Deps), + } + fullargs = append(fullargs, cfg.Flags...) fullargs = append(fullargs, "--") fullargs = append(fullargs, words...) return fullargs } // golist returns the JSON-encoded result of a "go list args..." query. -func golist(cfg *rawConfig, args []string) (*bytes.Buffer, error) { +func golist(ctx context.Context, cfg *rawConfig, args []string) (*bytes.Buffer, error) { out := new(bytes.Buffer) - cmd := exec.CommandContext(cfg.Context, "go", args...) + cmd := exec.CommandContext(ctx, "go", args...) cmd.Env = cfg.Env cmd.Dir = cfg.Dir cmd.Stdout = out diff --git a/go/packages/golist_fallback.go b/go/packages/golist_fallback.go index b84dda39..a06aab21 100644 --- a/go/packages/golist_fallback.go +++ b/go/packages/golist_fallback.go @@ -1,6 +1,7 @@ package packages import ( + "context" "encoding/json" "fmt" @@ -18,19 +19,21 @@ import ( // TODO(matloob): Support cgo. Copy code from the loader that runs cgo. -func golistPackagesFallback(cfg *rawConfig, words ...string) ([]*rawPackage, error) { - original, deps, err := getDeps(cfg, words...) +func golistPackagesFallback(ctx context.Context, cfg *rawConfig, words ...string) ([]string, []*rawPackage, error) { + original, deps, err := getDeps(ctx, cfg, words...) if err != nil { - return nil, err + return nil, nil, err } var result []*rawPackage + var roots []string addPackage := func(p *jsonPackage) { if p.Name == "" { return } id := p.ImportPath + isRoot := original[id] != nil pkgpath := id if pkgpath == "unsafe" { @@ -53,7 +56,9 @@ func golistPackagesFallback(cfg *rawConfig, words ...string) ([]*rawPackage, err } return importMap } - + if isRoot { + roots = append(roots, id) + } result = append(result, &rawPackage{ ID: id, Name: p.Name, @@ -61,11 +66,13 @@ func golistPackagesFallback(cfg *rawConfig, words ...string) ([]*rawPackage, err OtherFiles: absJoin(p.Dir, p.SFiles, p.CFiles), PkgPath: pkgpath, Imports: importMap(p.Imports), - DepOnly: original[id] == nil, }) if cfg.Tests { testID := fmt.Sprintf("%s [%s.test]", id, id) if len(p.TestGoFiles) > 0 || len(p.XTestGoFiles) > 0 { + if isRoot { + roots = append(roots, testID) + } result = append(result, &rawPackage{ ID: testID, Name: p.Name, @@ -73,17 +80,19 @@ func golistPackagesFallback(cfg *rawConfig, words ...string) ([]*rawPackage, err OtherFiles: absJoin(p.Dir, p.SFiles, p.CFiles), PkgPath: pkgpath, Imports: importMap(append(p.Imports, p.TestImports...)), - DepOnly: original[id] == nil, }) } if len(p.XTestGoFiles) > 0 { + xtestID := fmt.Sprintf("%s_test [%s.test]", id, id) + if isRoot { + roots = append(roots, xtestID) + } result = append(result, &rawPackage{ - ID: fmt.Sprintf("%s_test [%s.test]", id, id), + ID: xtestID, Name: p.Name + "_test", GoFiles: absJoin(p.Dir, p.XTestGoFiles), PkgPath: pkgpath, Imports: importMap(append(p.XTestImports, testID)), - DepOnly: original[id] == nil, }) } } @@ -93,30 +102,30 @@ func golistPackagesFallback(cfg *rawConfig, words ...string) ([]*rawPackage, err addPackage(pkg) } if !cfg.Deps || len(deps) == 0 { - return result, nil + return roots, result, nil } - buf, err := golist(cfg, golistargs_fallback(cfg, deps)) + buf, err := golist(ctx, cfg, golistargs_fallback(cfg, deps)) if err != nil { - return nil, err + return nil, nil, err } // Decode the JSON and convert it to rawPackage form. for dec := json.NewDecoder(buf); dec.More(); { p := new(jsonPackage) if err := dec.Decode(p); err != nil { - return nil, fmt.Errorf("JSON decoding failed: %v", err) + return nil, nil, fmt.Errorf("JSON decoding failed: %v", err) } addPackage(p) } - return result, nil + return roots, result, nil } // getDeps runs an initial go list to determine all the dependency packages. -func getDeps(cfg *rawConfig, words ...string) (originalSet map[string]*jsonPackage, deps []string, err error) { - buf, err := golist(cfg, golistargs_fallback(cfg, words)) +func getDeps(ctx context.Context, cfg *rawConfig, words ...string) (originalSet map[string]*jsonPackage, deps []string, err error) { + buf, err := golist(ctx, cfg, golistargs_fallback(cfg, words)) if err != nil { return nil, nil, err } @@ -149,7 +158,7 @@ func getDeps(cfg *rawConfig, words ...string) (originalSet map[string]*jsonPacka func golistargs_fallback(cfg *rawConfig, words []string) []string { fullargs := []string{"list", "-e", "-json"} - fullargs = append(fullargs, cfg.ExtraFlags...) + fullargs = append(fullargs, cfg.Flags...) fullargs = append(fullargs, "--") fullargs = append(fullargs, words...) return fullargs diff --git a/go/packages/packages.go b/go/packages/packages.go index a1ceb8b9..858e554a 100644 --- a/go/packages/packages.go +++ b/go/packages/packages.go @@ -295,10 +295,10 @@ func (ld *loader) load(patterns ...string) ([]*Package, error) { listfunc := golistPackages // TODO(matloob): Patterns may now be empty, if it was solely comprised of contains: patterns. // See if the extra process invocation can be avoided. - list, err := listfunc(rawCfg, patterns...) + roots, list, err := listfunc(ld.Context, rawCfg, patterns...) if _, ok := err.(GoTooOldError); ok { listfunc = golistPackagesFallback - list, err = listfunc(rawCfg, patterns...) + roots, list, err = listfunc(ld.Context, rawCfg, patterns...) } if err != nil { return nil, err @@ -315,7 +315,7 @@ func (ld *loader) load(patterns ...string) ([]*Package, error) { // TODO(matloob): Do only one query per directory. fdir := filepath.Dir(f) rawCfg.Dir = fdir - cList, err := listfunc(rawCfg, ".") + _, cList, err := listfunc(ld.Context, rawCfg, ".") if err != nil { return nil, err } @@ -327,10 +327,9 @@ func (ld *loader) load(patterns ...string) ([]*Package, error) { } seenPkgs[pkg.ID] = true dedupedList = append(dedupedList, pkg) - pkg.DepOnly = true for _, pkgFile := range pkg.GoFiles { if filepath.Base(f) == filepath.Base(pkgFile) { - pkg.DepOnly = false + roots = append(roots, pkg.ID) break } } @@ -338,12 +337,17 @@ func (ld *loader) load(patterns ...string) ([]*Package, error) { list = append(list, dedupedList...) } - return ld.loadFrom(list...) + return ld.loadFrom(roots, list...) } -func (ld *loader) loadFrom(list ...*rawPackage) ([]*Package, error) { +func (ld *loader) loadFrom(roots []string, list ...*rawPackage) ([]*Package, error) { + if len(list) == 0 { + return nil, fmt.Errorf("packages not found") + } + if len(roots) == 0 { + return nil, fmt.Errorf("packages had no initial set") + } ld.pkgs = make(map[string]*loaderPackage, len(list)) - var initial []*loaderPackage // first pass, fixup and build the map and roots for _, pkg := range list { lpkg := &loaderPackage{ @@ -357,18 +361,19 @@ func (ld *loader) loadFrom(list ...*rawPackage) ([]*Package, error) { needsrc: ld.Mode >= LoadAllSyntax || pkg.Export == "", } ld.pkgs[lpkg.ID] = lpkg - if !pkg.DepOnly { - initial = append(initial, lpkg) - if ld.Mode == LoadSyntax { - lpkg.needsrc = true - } + } + // check all the roots were found + initial := make([]*loaderPackage, len(roots)) + for i, root := range roots { + lpkg := ld.pkgs[root] + if lpkg == nil { + return nil, fmt.Errorf("root package %v not found", root) + } + initial[i] = lpkg + // mark the roots as needing source + 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. diff --git a/go/packages/raw.go b/go/packages/raw.go index 58a66eb1..f13ecae6 100644 --- a/go/packages/raw.go +++ b/go/packages/raw.go @@ -5,8 +5,6 @@ package packages import ( - "context" - "fmt" "os" ) @@ -51,37 +49,25 @@ type rawPackage struct { // rawConfig specifies details about what raw package information is needed // and how the underlying build tool should load package data. type rawConfig struct { - Context context.Context - Dir string - Env []string - ExtraFlags []string - Export bool - Tests bool - Deps bool + Dir string + Env []string + Flags []string + Export bool + Tests bool + Deps bool } func newRawConfig(cfg *Config) *rawConfig { rawCfg := &rawConfig{ - Context: cfg.Context, - Dir: cfg.Dir, - Env: cfg.Env, - ExtraFlags: cfg.Flags, - Export: cfg.Mode > LoadImports && cfg.Mode < LoadAllSyntax, - Tests: cfg.Tests, - Deps: cfg.Mode >= LoadImports, + Dir: cfg.Dir, + Env: cfg.Env, + Flags: cfg.Flags, + Export: cfg.Mode > LoadImports && cfg.Mode < LoadAllSyntax, + Tests: cfg.Tests, + Deps: cfg.Mode >= LoadImports, } if rawCfg.Env == nil { rawCfg.Env = os.Environ() } return rawCfg } - -func (cfg *rawConfig) Flags() []string { - return append([]string{ - fmt.Sprintf("-test=%t", cfg.Tests), - fmt.Sprintf("-export=%t", cfg.Export), - fmt.Sprintf("-deps=%t", cfg.Deps), - }, - cfg.ExtraFlags..., - ) -}