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 <iancottrell@google.com>
Run-TryBot: Michael Matloob <matloob@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Michael Matloob <matloob@golang.org>
This commit is contained in:
Ian Cottrell 2018-10-30 10:53:58 -04:00
parent 6c7e314b65
commit 3a10b9bf0a
3 changed files with 13 additions and 12 deletions

View File

@ -20,7 +20,6 @@ import (
"os" "os"
"path/filepath" "path/filepath"
"runtime" "runtime"
"sort"
"strings" "strings"
"sync" "sync"
@ -174,7 +173,6 @@ func Load(cfg *Config, patterns ...string) ([]*Package, error) {
if err != nil { if err != nil {
return nil, err return nil, err
} }
sort.Strings(response.Roots) // make all driver responses deterministic
return l.refine(response.Roots, response.Packages...) 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 // refine connects the supplied packages into a graph and then adds type and
// and syntax information as requested by the LoadMode. // and syntax information as requested by the LoadMode.
func (ld *loader) refine(roots []string, list ...*Package) ([]*Package, error) { func (ld *loader) refine(roots []string, list ...*Package) ([]*Package, error) {
isRoot := make(map[string]bool, len(roots)) rootMap := make(map[string]int, len(roots))
for _, root := range roots { for i, root := range roots {
isRoot[root] = true rootMap[root] = i
} }
ld.pkgs = make(map[string]*loaderPackage) ld.pkgs = make(map[string]*loaderPackage)
// first pass, fixup and build the map and roots // first pass, fixup and build the map and roots
var initial []*loaderPackage var initial = make([]*loaderPackage, len(roots))
for _, pkg := range list { for _, pkg := range list {
rootIndex := -1
if i, found := rootMap[pkg.ID]; found {
rootIndex = i
}
lpkg := &loaderPackage{ lpkg := &loaderPackage{
Package: pkg, Package: pkg,
needtypes: ld.Mode >= LoadAllSyntax || needtypes: ld.Mode >= LoadAllSyntax ||
ld.Mode >= LoadTypes && isRoot[pkg.ID], ld.Mode >= LoadTypes && rootIndex >= 0,
needsrc: ld.Mode >= LoadAllSyntax || needsrc: ld.Mode >= LoadAllSyntax ||
ld.Mode >= LoadSyntax && isRoot[pkg.ID] || ld.Mode >= LoadSyntax && rootIndex >= 0 ||
pkg.ExportFile == "" && pkg.PkgPath != "unsafe", pkg.ExportFile == "" && pkg.PkgPath != "unsafe",
} }
ld.pkgs[lpkg.ID] = lpkg ld.pkgs[lpkg.ID] = lpkg
if isRoot[lpkg.ID] { if rootIndex >= 0 {
initial = append(initial, lpkg) initial[rootIndex] = lpkg
lpkg.initial = true lpkg.initial = true
} }
} }

View File

@ -298,6 +298,7 @@ func TestLoadAbsolutePath(t *testing.T) {
for _, p := range initial { for _, p := range initial {
got = append(got, p.ID) got = append(got, p.ID)
} }
sort.Strings(got)
want := []string{"golang.org/gopatha/a", "golang.org/gopathb/b"} want := []string{"golang.org/gopatha/a", "golang.org/gopathb/b"}
if !reflect.DeepEqual(got, want) { if !reflect.DeepEqual(got, want) {
t.Fatalf("initial packages loaded: got [%s], want [%s]", got, want) t.Fatalf("initial packages loaded: got [%s], want [%s]", got, want)

View File

@ -3,7 +3,6 @@ package packages
import ( import (
"fmt" "fmt"
"os" "os"
"sort"
) )
// Visit visits all the packages in the import graph whose roots are // 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 { for path := range pkg.Imports {
paths = append(paths, path) paths = append(paths, path)
} }
sort.Strings(paths) // for determinism
for _, path := range paths { for _, path := range paths {
visit(pkg.Imports[path]) visit(pkg.Imports[path])
} }