From 3a10b9bf0a52df7e992a8c3eb712a86d3c896c75 Mon Sep 17 00:00:00 2001 From: Ian Cottrell Date: Tue, 30 Oct 2018 10:53:58 -0400 Subject: [PATCH] go/packages: change so no results are sorted We do insist that drivers are stable (not sorted, just stable) and that refine is also stable, which allows us to promise a stable output. I also changed refine so it returns the root set in the same order that the original root id list was supplied, as this seems to be a strictly better experience. Change-Id: I8eb0bffd7547865d14a6c6f18646018b9af140bd Reviewed-on: https://go-review.googlesource.com/c/145877 Run-TryBot: Ian Cottrell Run-TryBot: Michael Matloob TryBot-Result: Gobot Gobot Reviewed-by: Michael Matloob --- go/packages/packages.go | 22 ++++++++++++---------- go/packages/packages_test.go | 1 + go/packages/visit.go | 2 -- 3 files changed, 13 insertions(+), 12 deletions(-) diff --git a/go/packages/packages.go b/go/packages/packages.go index ae70c22b..4e78f341 100644 --- a/go/packages/packages.go +++ b/go/packages/packages.go @@ -20,7 +20,6 @@ import ( "os" "path/filepath" "runtime" - "sort" "strings" "sync" @@ -174,7 +173,6 @@ func Load(cfg *Config, patterns ...string) ([]*Package, error) { if err != nil { return nil, err } - sort.Strings(response.Roots) // make all driver responses deterministic return l.refine(response.Roots, response.Packages...) } @@ -413,25 +411,29 @@ func newLoader(cfg *Config) *loader { // refine connects the supplied packages into a graph and then adds type and // and syntax information as requested by the LoadMode. func (ld *loader) refine(roots []string, list ...*Package) ([]*Package, error) { - isRoot := make(map[string]bool, len(roots)) - for _, root := range roots { - isRoot[root] = true + rootMap := make(map[string]int, len(roots)) + for i, root := range roots { + rootMap[root] = i } ld.pkgs = make(map[string]*loaderPackage) // first pass, fixup and build the map and roots - var initial []*loaderPackage + var initial = make([]*loaderPackage, len(roots)) for _, pkg := range list { + rootIndex := -1 + if i, found := rootMap[pkg.ID]; found { + rootIndex = i + } lpkg := &loaderPackage{ Package: pkg, needtypes: ld.Mode >= LoadAllSyntax || - ld.Mode >= LoadTypes && isRoot[pkg.ID], + ld.Mode >= LoadTypes && rootIndex >= 0, needsrc: ld.Mode >= LoadAllSyntax || - ld.Mode >= LoadSyntax && isRoot[pkg.ID] || + ld.Mode >= LoadSyntax && rootIndex >= 0 || pkg.ExportFile == "" && pkg.PkgPath != "unsafe", } ld.pkgs[lpkg.ID] = lpkg - if isRoot[lpkg.ID] { - initial = append(initial, lpkg) + if rootIndex >= 0 { + initial[rootIndex] = lpkg lpkg.initial = true } } diff --git a/go/packages/packages_test.go b/go/packages/packages_test.go index 6a3eaecb..3f800036 100644 --- a/go/packages/packages_test.go +++ b/go/packages/packages_test.go @@ -298,6 +298,7 @@ func TestLoadAbsolutePath(t *testing.T) { for _, p := range initial { got = append(got, p.ID) } + sort.Strings(got) want := []string{"golang.org/gopatha/a", "golang.org/gopathb/b"} if !reflect.DeepEqual(got, want) { t.Fatalf("initial packages loaded: got [%s], want [%s]", got, want) diff --git a/go/packages/visit.go b/go/packages/visit.go index c1a4b28c..a50eb3b5 100644 --- a/go/packages/visit.go +++ b/go/packages/visit.go @@ -3,7 +3,6 @@ package packages import ( "fmt" "os" - "sort" ) // Visit visits all the packages in the import graph whose roots are @@ -24,7 +23,6 @@ func Visit(pkgs []*Package, pre func(*Package) bool, post func(*Package)) { for path := range pkg.Imports { paths = append(paths, path) } - sort.Strings(paths) // for determinism for _, path := range paths { visit(pkg.Imports[path]) }