diff --git a/cmd/dep/ensure.go b/cmd/dep/ensure.go index ef35e7f069..4b4c623052 100644 --- a/cmd/dep/ensure.go +++ b/cmd/dep/ensure.go @@ -178,12 +178,12 @@ func (cmd *ensureCommand) Run(ctx *dep.Ctx, args []string) error { return cmd.runVendorOnly(ctx, args, p, sm, params) } - params.RootPackageTree, err = pkgtree.ListPackages(p.ResolvedAbsRoot, string(p.ImportRoot)) + params.RootPackageTree, err = p.ParseRootPackageTree() if err != nil { - return errors.Wrap(err, "ensure ListPackage for project") + return err } - if fatal, err := checkErrors(params.RootPackageTree.Packages); err != nil { + if fatal, err := checkErrors(params.RootPackageTree.Packages, p.Manifest.IgnoredPackages()); err != nil { if fatal { return err } else if ctx.Verbose { @@ -761,13 +761,17 @@ func getProjectConstraint(arg string, sm gps.SourceManager) (gps.ProjectConstrai return gps.ProjectConstraint{Ident: pi, Constraint: c}, arg, nil } -func checkErrors(m map[string]pkgtree.PackageOrErr) (fatal bool, err error) { +func checkErrors(m map[string]pkgtree.PackageOrErr, ignore *pkgtree.IgnoredRuleset) (fatal bool, err error) { var ( noGoErrors int pkgtreeErrors = make(pkgtreeErrs, 0, len(m)) ) - for _, poe := range m { + for ip, poe := range m { + if ignore.IsIgnored(ip) { + continue + } + if poe.Err != nil { switch poe.Err.(type) { case *build.NoGoError: diff --git a/cmd/dep/ensure_test.go b/cmd/dep/ensure_test.go index c4fb0c7a24..e8cf300e87 100644 --- a/cmd/dep/ensure_test.go +++ b/cmd/dep/ensure_test.go @@ -135,7 +135,7 @@ func TestCheckErrors(t *testing.T) { for _, tc := range tt { t.Run(tc.name, func(t *testing.T) { - fatal, err := checkErrors(tc.pkgOrErrMap) + fatal, err := checkErrors(tc.pkgOrErrMap, nil) if tc.fatal != fatal { t.Fatalf("expected fatal flag to be %T, got %T", tc.fatal, fatal) } diff --git a/cmd/dep/init.go b/cmd/dep/init.go index 4cebf4ae34..a4f6637d55 100644 --- a/cmd/dep/init.go +++ b/cmd/dep/init.go @@ -227,9 +227,9 @@ func (cmd *initCommand) Run(ctx *dep.Ctx, args []string) error { } func getDirectDependencies(sm gps.SourceManager, p *dep.Project) (pkgtree.PackageTree, map[string]bool, error) { - pkgT, err := pkgtree.ListPackages(p.ResolvedAbsRoot, string(p.ImportRoot)) + pkgT, err := p.ParseRootPackageTree() if err != nil { - return pkgtree.PackageTree{}, nil, errors.Wrap(err, "gps.ListPackages") + return pkgtree.PackageTree{}, nil, err } directDeps := map[string]bool{} diff --git a/cmd/dep/status.go b/cmd/dep/status.go index 93b1e73a10..ff5355f700 100644 --- a/cmd/dep/status.go +++ b/cmd/dep/status.go @@ -171,7 +171,8 @@ type dotOutput struct { func (out *dotOutput) BasicHeader() { out.g = new(graphviz).New() - ptree, _ := pkgtree.ListPackages(out.p.ResolvedAbsRoot, string(out.p.ImportRoot)) + ptree, _ := out.p.ParseRootPackageTree() + // TODO(sdboyer) should be true, true, false, out.p.Manifest.IgnoredPackages() prm, _ := ptree.ToReachMap(true, false, false, nil) out.g.createNode(string(out.p.ImportRoot), "", prm.FlattenFn(paths.IsStandardImportPath)) @@ -358,9 +359,9 @@ type MissingStatus struct { func runStatusAll(ctx *dep.Ctx, out outputter, p *dep.Project, sm gps.SourceManager) (hasMissingPkgs bool, errCount int, err error) { // While the network churns on ListVersions() requests, statically analyze // code from the current project. - ptree, err := pkgtree.ListPackages(p.ResolvedAbsRoot, string(p.ImportRoot)) + ptree, err := p.ParseRootPackageTree() if err != nil { - return false, 0, errors.Wrapf(err, "analysis of local packages failed") + return false, 0, err } // Set up a solver in order to check the InputHash. @@ -437,7 +438,7 @@ func runStatusAll(ctx *dep.Ctx, out outputter, p *dep.Project, sm gps.SourceMana errListPkgCh <- err } - prm, _ := ptr.ToReachMap(true, false, false, nil) + prm, _ := ptr.ToReachMap(true, true, false, p.Manifest.IgnoredPackages()) bs.Children = prm.FlattenFn(paths.IsStandardImportPath) } diff --git a/internal/gps/_testdata/src/varied_hidden/.onlyfromtests/a.go b/internal/gps/_testdata/src/varied_hidden/.onlyfromtests/a.go new file mode 100644 index 0000000000..66e136f943 --- /dev/null +++ b/internal/gps/_testdata/src/varied_hidden/.onlyfromtests/a.go @@ -0,0 +1,18 @@ +// Copyright 2017 The Go Authors. All rights reserved. +// Use of this source code is governed by a BSD-style +// license that can be found in the LICENSE file. + +package onlyfromtests + +import ( + "sort" + + _ "varied/_secondorder" + + "github.com/golang/dep/internal/gps" +) + +var ( + M = sort.Strings + _ = gps.Solve +) diff --git a/internal/gps/_testdata/src/varied_hidden/.onlyfromtests/b.go b/internal/gps/_testdata/src/varied_hidden/.onlyfromtests/b.go new file mode 100644 index 0000000000..fa353864bf --- /dev/null +++ b/internal/gps/_testdata/src/varied_hidden/.onlyfromtests/b.go @@ -0,0 +1,15 @@ +// Copyright 2017 The Go Authors. All rights reserved. +// Use of this source code is governed by a BSD-style +// license that can be found in the LICENSE file. + +package onlyfromtests + +import ( + "os" + "sort" +) + +var ( + _ = sort.Strings + _ = os.PathSeparator +) diff --git a/internal/gps/_testdata/src/varied_hidden/_frommain/a.go b/internal/gps/_testdata/src/varied_hidden/_frommain/a.go new file mode 100644 index 0000000000..28ee51be7a --- /dev/null +++ b/internal/gps/_testdata/src/varied_hidden/_frommain/a.go @@ -0,0 +1,16 @@ +// Copyright 2017 The Go Authors. All rights reserved. +// Use of this source code is governed by a BSD-style +// license that can be found in the LICENSE file. + +package frommain + +import ( + "sort" + + "github.com/golang/dep/internal/gps" +) + +var ( + M = sort.Strings + _ = gps.Solve +) diff --git a/internal/gps/_testdata/src/varied_hidden/_never/a.go b/internal/gps/_testdata/src/varied_hidden/_never/a.go new file mode 100644 index 0000000000..72c7c99245 --- /dev/null +++ b/internal/gps/_testdata/src/varied_hidden/_never/a.go @@ -0,0 +1,16 @@ +// Copyright 2017 The Go Authors. All rights reserved. +// Use of this source code is governed by a BSD-style +// license that can be found in the LICENSE file. + +package never + +import ( + "sort" + + "github.com/golang/dep/internal/gps" +) + +var ( + M = sort.Strings + _ = gps.Solve +) diff --git a/internal/gps/_testdata/src/varied_hidden/_secondorder/secondorder.go b/internal/gps/_testdata/src/varied_hidden/_secondorder/secondorder.go new file mode 100644 index 0000000000..4224b81e1e --- /dev/null +++ b/internal/gps/_testdata/src/varied_hidden/_secondorder/secondorder.go @@ -0,0 +1,11 @@ +// Copyright 2017 The Go Authors. All rights reserved. +// Use of this source code is governed by a BSD-style +// license that can be found in the LICENSE file. + +package secondorder + +import "hash" + +var ( + H = hash.Hash +) diff --git a/internal/gps/_testdata/src/varied_hidden/always/always_test.go b/internal/gps/_testdata/src/varied_hidden/always/always_test.go new file mode 100644 index 0000000000..043805b4bf --- /dev/null +++ b/internal/gps/_testdata/src/varied_hidden/always/always_test.go @@ -0,0 +1,7 @@ +// Copyright 2017 The Go Authors. All rights reserved. +// Use of this source code is governed by a BSD-style +// license that can be found in the LICENSE file. + +package always + +import _ "varied/.onlyfromtests" diff --git a/internal/gps/_testdata/src/varied_hidden/dotdotslash/a.go b/internal/gps/_testdata/src/varied_hidden/dotdotslash/a.go new file mode 100644 index 0000000000..f1ffb55bdd --- /dev/null +++ b/internal/gps/_testdata/src/varied_hidden/dotdotslash/a.go @@ -0,0 +1,13 @@ +// Copyright 2017 The Go Authors. All rights reserved. +// Use of this source code is governed by a BSD-style +// license that can be found in the LICENSE file. + +package dotslash + +import ( + "../github.com/golang/dep/internal/gps" +) + +var ( + A = gps.Solver +) diff --git a/internal/gps/_testdata/src/varied_hidden/locals.go b/internal/gps/_testdata/src/varied_hidden/locals.go new file mode 100644 index 0000000000..4afaa8bfd3 --- /dev/null +++ b/internal/gps/_testdata/src/varied_hidden/locals.go @@ -0,0 +1,14 @@ +// Copyright 2017 The Go Authors. All rights reserved. +// Use of this source code is governed by a BSD-style +// license that can be found in the LICENSE file. + +package main + +import ( + _ "varied/_frommain" + "varied/simple" +) + +var ( + _ = simple.S +) diff --git a/internal/gps/_testdata/src/varied_hidden/main.go b/internal/gps/_testdata/src/varied_hidden/main.go new file mode 100644 index 0000000000..0812e3ca60 --- /dev/null +++ b/internal/gps/_testdata/src/varied_hidden/main.go @@ -0,0 +1,13 @@ +// Copyright 2017 The Go Authors. All rights reserved. +// Use of this source code is governed by a BSD-style +// license that can be found in the LICENSE file. + +package main + +import ( + "net/http" +) + +var ( + _ = http.Client +) diff --git a/internal/gps/_testdata/src/varied_hidden/simple/locals.go b/internal/gps/_testdata/src/varied_hidden/simple/locals.go new file mode 100644 index 0000000000..7d682c4e75 --- /dev/null +++ b/internal/gps/_testdata/src/varied_hidden/simple/locals.go @@ -0,0 +1,11 @@ +// Copyright 2017 The Go Authors. All rights reserved. +// Use of this source code is governed by a BSD-style +// license that can be found in the LICENSE file. + +package simple + +import "varied/simple/testdata" + +var ( + _ = testdata.H +) diff --git a/internal/gps/_testdata/src/varied_hidden/simple/simple.go b/internal/gps/_testdata/src/varied_hidden/simple/simple.go new file mode 100644 index 0000000000..9d0c3445f6 --- /dev/null +++ b/internal/gps/_testdata/src/varied_hidden/simple/simple.go @@ -0,0 +1,16 @@ +// Copyright 2017 The Go Authors. All rights reserved. +// Use of this source code is governed by a BSD-style +// license that can be found in the LICENSE file. + +package simple + +import ( + "go/parser" + + "github.com/golang/dep/internal/gps" +) + +var ( + _ = parser.ParseFile + S = gps.Prepare +) diff --git a/internal/gps/_testdata/src/varied_hidden/simple/testdata/another.go b/internal/gps/_testdata/src/varied_hidden/simple/testdata/another.go new file mode 100644 index 0000000000..02eab2a200 --- /dev/null +++ b/internal/gps/_testdata/src/varied_hidden/simple/testdata/another.go @@ -0,0 +1,7 @@ +// Copyright 2017 The Go Authors. All rights reserved. +// Use of this source code is governed by a BSD-style +// license that can be found in the LICENSE file. + +package testdata + +import _ "varied/dotdotslash" diff --git a/internal/gps/pkgtree/ignored_ruleset.go b/internal/gps/pkgtree/ignored_ruleset.go new file mode 100644 index 0000000000..30b92bd8e9 --- /dev/null +++ b/internal/gps/pkgtree/ignored_ruleset.go @@ -0,0 +1,111 @@ +// Copyright 2017 The Go Authors. All rights reserved. +// Use of this source code is governed by a BSD-style +// license that can be found in the LICENSE file. + +package pkgtree + +import ( + "sort" + "strings" + + "github.com/armon/go-radix" +) + +// IgnoredRuleset comprises a set of rules for ignoring import paths. It can +// manage both literal and prefix-wildcard matches. +type IgnoredRuleset struct { + t *radix.Tree +} + +// NewIgnoredRuleset processes a set of strings into an IgnoredRuleset. Strings +// that end in "*" are treated as wildcards, where any import path with a +// matching prefix will be ignored. IgnoredRulesets are immutable once created. +// +// Duplicate and redundant (i.e. a literal path that has a prefix of a wildcard +// path) declarations are discarded. Consequently, it is possible that the +// returned IgnoredRuleset may have a smaller Len() than the input slice. +func NewIgnoredRuleset(ig []string) *IgnoredRuleset { + if len(ig) == 0 { + return &IgnoredRuleset{} + } + + ir := &IgnoredRuleset{ + t: radix.New(), + } + + // Sort the list of all the ignores in order to ensure that wildcard + // precedence is recorded correctly in the trie. + sort.Strings(ig) + for _, i := range ig { + // Skip global ignore and empty string. + if i == "*" || i == "" { + continue + } + + _, wildi, has := ir.t.LongestPrefix(i) + // We may not always have a value here, but if we do, then it's a bool. + wild, _ := wildi.(bool) + // Check if it's a wildcard ignore. + if strings.HasSuffix(i, "*") { + // Check if it is ineffectual. + if has && wild { + // Skip ineffectual wildcard ignore. + continue + } + // Create the ignore prefix and insert in the radix tree. + ir.t.Insert(i[:len(i)-1], true) + } else if !has || !wild { + ir.t.Insert(i, false) + } + } + + if ir.t.Len() == 0 { + ir.t = nil + } + + return ir +} + +// IsIgnored indicates whether the provided path should be ignored, according to +// the ruleset. +func (ir *IgnoredRuleset) IsIgnored(path string) bool { + if path == "" || ir == nil || ir.t == nil { + return false + } + + prefix, wildi, has := ir.t.LongestPrefix(path) + return has && (wildi.(bool) || path == prefix) +} + +// Len indicates the number of rules in the ruleset. +func (ir *IgnoredRuleset) Len() int { + if ir == nil || ir.t == nil { + return 0 + } + + return ir.t.Len() +} + +// ToSlice converts the contents of the IgnoredRuleset to a string slice. +// +// This operation is symmetrically dual to NewIgnoredRuleset. +func (ir *IgnoredRuleset) ToSlice() []string { + irlen := ir.Len() + if irlen == 0 { + return nil + } + + items := make([]string, 0, irlen) + ir.t.Walk(func(s string, v interface{}) bool { + if s != "" { + if v.(bool) { + items = append(items, s+"*") + } else { + items = append(items, s) + } + } + return false + }) + + return items +} diff --git a/internal/gps/pkgtree/ignored_ruleset_test.go b/internal/gps/pkgtree/ignored_ruleset_test.go new file mode 100644 index 0000000000..0dab4309c4 --- /dev/null +++ b/internal/gps/pkgtree/ignored_ruleset_test.go @@ -0,0 +1,204 @@ +// Copyright 2017 The Go Authors. All rights reserved. +// Use of this source code is governed by a BSD-style +// license that can be found in the LICENSE file. + +package pkgtree + +import "testing" + +func TestIgnoredRuleset(t *testing.T) { + type tfixm []struct { + path string + wild bool + } + cases := []struct { + name string + inputs []string + wantInTree tfixm + wantEmptyTree bool + shouldIgnore []string + shouldNotIgnore []string + }{ + { + name: "only skip global ignore", + inputs: []string{"*"}, + wantEmptyTree: true, + }, + { + name: "ignores without ignore suffix", + inputs: []string{ + "x/y/z", + "*a/b/c", + "gophers", + }, + wantInTree: tfixm{ + {path: "x/y/z", wild: false}, + {path: "*a/b/c", wild: false}, + {path: "gophers", wild: false}, + }, + shouldIgnore: []string{ + "x/y/z", + "gophers", + }, + shouldNotIgnore: []string{ + "x/y/z/q", + "x/y", + "gopher", + "gopherss", + }, + }, + { + name: "ignores with ignore suffix", + inputs: []string{ + "x/y/z*", + "a/b/c", + "gophers", + }, + wantInTree: tfixm{ + {path: "x/y/z", wild: true}, + {path: "a/b/c", wild: false}, + {path: "gophers", wild: false}, + }, + shouldIgnore: []string{ + "x/y/z", + "x/y/zz", + "x/y/z/", + "x/y/z/q", + }, + shouldNotIgnore: []string{ + "x/y", + "gopher", + }, + }, + { + name: "global ignore with other strings", + inputs: []string{ + "*", + "gophers*", + "x/y/z*", + "a/b/c", + }, + wantInTree: tfixm{ + {path: "x/y/z", wild: true}, + {path: "a/b/c", wild: false}, + {path: "gophers", wild: true}, + }, + shouldIgnore: []string{ + "x/y/z", + "x/y/z/", + "x/y/z/q", + "gophers", + "gopherss", + "gophers/foo", + }, + shouldNotIgnore: []string{ + "x/y", + "gopher", + }, + }, + { + name: "ineffectual ignore with wildcard", + inputs: []string{ + "a/b*", + "a/b/c*", + "a/b/x/y", + "a/c*", + }, + wantInTree: tfixm{ + {path: "a/c", wild: true}, + {path: "a/b", wild: true}, + }, + shouldIgnore: []string{ + "a/cb", + }, + shouldNotIgnore: []string{ + "a/", + "a/d", + }, + }, + { + name: "same path with and without wildcard", + inputs: []string{ + "a/b*", + "a/b", + }, + wantInTree: tfixm{ + {path: "a/b", wild: true}, + }, + shouldIgnore: []string{ + "a/b", + "a/bb", + }, + shouldNotIgnore: []string{ + "a/", + "a/d", + }, + }, + { + name: "empty paths", + inputs: []string{ + "", + "a/b*", + }, + wantInTree: tfixm{ + {path: "a/b", wild: true}, + }, + shouldNotIgnore: []string{ + "", + }, + }, + { + name: "single wildcard", + inputs: []string{ + "a/b*", + }, + wantInTree: tfixm{ + {path: "a/b", wild: true}, + }, + shouldIgnore: []string{ + "a/b/c", + }, + }, + } + + for _, c := range cases { + igm := NewIgnoredRuleset(c.inputs) + f := func(t *testing.T) { + + if c.wantEmptyTree { + if igm.Len() != 0 { + t.Fatalf("wanted empty tree, but had %v elements", igm.Len()) + } + } + + // Check if the wildcard suffix ignores are in the tree. + for _, p := range c.wantInTree { + wildi, has := igm.t.Get(p.path) + if !has { + t.Fatalf("expected %q to be in the tree", p.path) + } else if wildi.(bool) != p.wild { + if p.wild { + t.Fatalf("expected %q to be a wildcard matcher, but it was not", p.path) + } else { + t.Fatalf("expected %q not to be a wildcard matcher, but it was", p.path) + } + } + } + + for _, p := range c.shouldIgnore { + if !igm.IsIgnored(p) { + t.Fatalf("%q should be ignored, but it was not", p) + } + } + for _, p := range c.shouldNotIgnore { + if igm.IsIgnored(p) { + t.Fatalf("%q should not be ignored, but it was", p) + } + } + } + t.Run(c.name, f) + + igm = NewIgnoredRuleset(igm.ToSlice()) + t.Run(c.name+"/inandout", f) + } +} diff --git a/internal/gps/pkgtree/pkgtree.go b/internal/gps/pkgtree/pkgtree.go index 78be8b42f4..f622d61156 100644 --- a/internal/gps/pkgtree/pkgtree.go +++ b/internal/gps/pkgtree/pkgtree.go @@ -14,12 +14,11 @@ import ( "go/token" "os" "path/filepath" + "reflect" "sort" "strconv" "strings" "unicode" - - "github.com/armon/go-radix" ) // Package represents a Go package. It contains a subset of the information @@ -140,20 +139,11 @@ func ListPackages(fileRoot, importRoot string) (PackageTree, error) { } err = fillPackage(p) - var pkg Package - if err == nil { - pkg = Package{ - ImportPath: ip, - CommentPath: p.ImportComment, - Name: p.Name, - Imports: p.Imports, - TestImports: dedupeStrings(p.TestImports, p.XTestImports), - } - } else { + if err != nil { switch err.(type) { - case gscan.ErrorList, *gscan.Error, *build.NoGoError: - // This happens if we encounter malformed or nonexistent Go - // source code + case gscan.ErrorList, *gscan.Error, *build.NoGoError, *ConflictingImportComments: + // Assorted cases in which we've encounter malformed or + // nonexistent Go source code. ptree.Packages[ip] = PackageOrErr{ Err: err, } @@ -163,6 +153,14 @@ func ListPackages(fileRoot, importRoot string) (PackageTree, error) { } } + pkg := Package{ + ImportPath: ip, + CommentPath: p.ImportComment, + Name: p.Name, + Imports: p.Imports, + TestImports: dedupeStrings(p.TestImports, p.XTestImports), + } + if pkg.CommentPath != "" && !strings.HasPrefix(pkg.CommentPath, importRoot) { ptree.Packages[ip] = PackageOrErr{ Err: &NonCanonicalImportRoot{ @@ -611,24 +609,108 @@ func (t PackageTree) ToReachMap(main, tests, backprop bool, ignore *IgnoredRules func (t PackageTree) Copy() PackageTree { t2 := PackageTree{ ImportRoot: t.ImportRoot, - Packages: map[string]PackageOrErr{}, + Packages: make(map[string]PackageOrErr, len(t.Packages)), + } + + // Walk through and count up the total number of string slice elements we'll + // need, then allocate them all at once. + strcount := 0 + for _, poe := range t.Packages { + strcount = strcount + len(poe.P.Imports) + len(poe.P.TestImports) } + pool := make([]string, strcount) for path, poe := range t.Packages { - poe2 := PackageOrErr{ - Err: poe.Err, - P: poe.P, + var poe2 PackageOrErr + + if poe.Err != nil { + refl := reflect.ValueOf(poe.Err) + switch refl.Kind() { + case reflect.Ptr: + poe2.Err = reflect.New(refl.Elem().Type()).Interface().(error) + case reflect.Slice: + err2 := reflect.MakeSlice(refl.Type(), refl.Len(), refl.Len()) + reflect.Copy(err2, refl) + poe2.Err = err2.Interface().(error) + default: + // This shouldn't be too onerous to maintain - the set of errors + // we can get here is restricted by what ListPackages() allows. + // So just panic if one is outside the expected kinds of ptr or + // slice, as that would mean we've missed something notable. + panic(fmt.Sprintf("unrecognized PackgeOrErr error type, %T", poe.Err)) + } + } else { + poe2.P = poe.P + il, til := len(poe.P.Imports), len(poe.P.TestImports) + if il > 0 { + poe2.P.Imports, pool = pool[:il], pool[il:] + copy(poe2.P.Imports, poe.P.Imports) + } + if til > 0 { + poe2.P.TestImports, pool = pool[:til], pool[til:] + copy(poe2.P.TestImports, poe.P.TestImports) + } } - if len(poe.P.Imports) > 0 { - poe2.P.Imports = make([]string, len(poe.P.Imports)) - copy(poe2.P.Imports, poe.P.Imports) + + t2.Packages[path] = poe2 + } + + return t2 +} + +// TrimHiddenPackages returns a new PackageTree where packages that are ignored, +// or both hidden and unreachable, have been removed. +// +// The package list is partitioned into two sets: visible, and hidden, where +// packages are considered hidden if they are within or beneath directories +// with: +// +// * leading dots +// * leading underscores +// * the exact name "testdata" +// +// Packages in the hidden set are dropped from the returned PackageTree, unless +// they are transitively reachable from imports in the visible set. +// +// The "main", "tests" and "ignored" parameters have the same behavior as with +// PackageTree.ToReachMap(): the first two determine, respectively, whether +// imports from main packages, and imports from tests, should be considered for +// reachability checks. Setting 'main' to true will additionally result in main +// packages being trimmed. +// +// "ignored" designates import paths, or patterns of import paths, where the +// corresponding packages should be excluded from reachability checks, if +// encountered. Ignored packages are also removed from the final set. +// +// Note that it is not recommended to call this method if the goal is to obtain +// a set of tree-external imports; calling ToReachMap and FlattenFn will achieve +// the same effect. +func (t PackageTree) TrimHiddenPackages(main, tests bool, ignore *IgnoredRuleset) PackageTree { + rm, pie := t.ToReachMap(main, tests, false, ignore) + t2 := t.Copy() + preserve := make(map[string]bool) + + for pkg, ie := range rm { + if pkgFilter(pkg) && !ignore.IsIgnored(pkg) { + preserve[pkg] = true + for _, in := range ie.Internal { + preserve[in] = true + } } - if len(poe.P.TestImports) > 0 { - poe2.P.TestImports = make([]string, len(poe.P.TestImports)) - copy(poe2.P.TestImports, poe.P.TestImports) + } + + // Also process the problem map, as packages in the visible set with errors + // need to be included in the return values. + for pkg := range pie { + if pkgFilter(pkg) && !ignore.IsIgnored(pkg) { + preserve[pkg] = true } + } - t2.Packages[path] = poe2 + for ip := range t.Packages { + if !preserve[ip] { + delete(t2.Packages, ip) + } } return t2 @@ -1024,102 +1106,3 @@ func uniq(a []string) []string { } return a[:i] } - -// IgnoredRuleset comprises a set of rules for ignoring import paths. It can -// manage both literal and prefix-wildcard matches. -type IgnoredRuleset struct { - t *radix.Tree -} - -// NewIgnoredRuleset processes a set of strings into an IgnoredRuleset. Strings -// that end in "*" are treated as wildcards, where any import path with a -// matching prefix will be ignored. IgnoredRulesets are immutable once created. -// -// Duplicate and redundant (i.e. a literal path that has a prefix of a wildcard -// path) declarations are discarded. Consequently, it is possible that the -// returned IgnoredRuleset may have a smaller Len() than the input slice. -func NewIgnoredRuleset(ig []string) *IgnoredRuleset { - if len(ig) == 0 { - return &IgnoredRuleset{} - } - - ir := &IgnoredRuleset{ - t: radix.New(), - } - - // Sort the list of all the ignores in order to ensure that wildcard - // precedence is recorded correctly in the trie. - sort.Strings(ig) - for _, i := range ig { - // Skip global ignore and empty string. - if i == "*" || i == "" { - continue - } - - _, wildi, has := ir.t.LongestPrefix(i) - // We may not always have a value here, but if we do, then it's a bool. - wild, _ := wildi.(bool) - // Check if it's a wildcard ignore. - if strings.HasSuffix(i, "*") { - // Check if it is ineffectual. - if has && wild { - // Skip ineffectual wildcard ignore. - continue - } - // Create the ignore prefix and insert in the radix tree. - ir.t.Insert(i[:len(i)-1], true) - } else if !has || !wild { - ir.t.Insert(i, false) - } - } - - if ir.t.Len() == 0 { - ir.t = nil - } - - return ir -} - -// IsIgnored indicates whether the provided path should be ignored, according to -// the ruleset. -func (ir *IgnoredRuleset) IsIgnored(path string) bool { - if path == "" || ir == nil || ir.t == nil { - return false - } - - prefix, wildi, has := ir.t.LongestPrefix(path) - return has && (wildi.(bool) || path == prefix) -} - -// Len indicates the number of rules in the ruleset. -func (ir *IgnoredRuleset) Len() int { - if ir == nil || ir.t == nil { - return 0 - } - - return ir.t.Len() -} - -// ToSlice converts the contents of the IgnoredRuleset to a string slice. -// -// This operation is symmetrically dual to NewIgnoredRuleset. -func (ir *IgnoredRuleset) ToSlice() []string { - irlen := ir.Len() - if irlen == 0 { - return nil - } - - items := make([]string, 0, irlen) - ir.t.Walk(func(s string, v interface{}) bool { - if s != "" { - if v.(bool) { - items = append(items, s+"*") - } else { - items = append(items, s) - } - } - return false - }) - - return items -} diff --git a/internal/gps/pkgtree/pkgtree_test.go b/internal/gps/pkgtree/pkgtree_test.go index bb97c11c6b..8b61a61d67 100644 --- a/internal/gps/pkgtree/pkgtree_test.go +++ b/internal/gps/pkgtree/pkgtree_test.go @@ -11,6 +11,7 @@ import ( "go/token" "io/ioutil" "os" + "path" "path/filepath" "reflect" "runtime" @@ -936,27 +937,6 @@ func TestListPackages(t *testing.T) { }, }, }, - // New code allows this because it doesn't care if the code compiles (kinda) or not, - // so maybe this is actually not an error anymore? - // - // TODO re-enable this case after the full and proper ListPackages() - // refactor in #99 - /*"two pkgs": { - fileRoot: j("twopkgs"), - importRoot: "twopkgs", - out: PackageTree{ - ImportRoot: "twopkgs", - Packages: map[string]PackageOrErr{ - "twopkgs": { - Err: &build.MultiplePackageError{ - Dir: j("twopkgs"), - Packages: []string{"simple", "m1p"}, - Files: []string{"a.go", "b.go"}, - }, - }, - }, - }, - }, */ // imports a missing pkg "missing import": { fileRoot: j("missing"), @@ -1229,6 +1209,114 @@ func TestListPackages(t *testing.T) { }, }, }, + "varied with hidden dirs": { + fileRoot: j("varied_hidden"), + importRoot: "varied", + out: PackageTree{ + ImportRoot: "varied", + Packages: map[string]PackageOrErr{ + "varied": { + P: Package{ + ImportPath: "varied", + CommentPath: "", + Name: "main", + Imports: []string{ + "net/http", + "varied/_frommain", + "varied/simple", + }, + }, + }, + "varied/always": { + P: Package{ + ImportPath: "varied/always", + CommentPath: "", + Name: "always", + Imports: []string{}, + TestImports: []string{ + "varied/.onlyfromtests", + }, + }, + }, + "varied/.onlyfromtests": { + P: Package{ + ImportPath: "varied/.onlyfromtests", + CommentPath: "", + Name: "onlyfromtests", + Imports: []string{ + "github.com/golang/dep/internal/gps", + "os", + "sort", + "varied/_secondorder", + }, + }, + }, + "varied/simple": { + P: Package{ + ImportPath: "varied/simple", + CommentPath: "", + Name: "simple", + Imports: []string{ + "github.com/golang/dep/internal/gps", + "go/parser", + "varied/simple/testdata", + }, + }, + }, + "varied/simple/testdata": { + P: Package{ + ImportPath: "varied/simple/testdata", + CommentPath: "", + Name: "testdata", + Imports: []string{ + "varied/dotdotslash", + }, + }, + }, + "varied/_secondorder": { + P: Package{ + ImportPath: "varied/_secondorder", + CommentPath: "", + Name: "secondorder", + Imports: []string{ + "hash", + }, + }, + }, + "varied/_never": { + P: Package{ + ImportPath: "varied/_never", + CommentPath: "", + Name: "never", + Imports: []string{ + "github.com/golang/dep/internal/gps", + "sort", + }, + }, + }, + "varied/_frommain": { + P: Package{ + ImportPath: "varied/_frommain", + CommentPath: "", + Name: "frommain", + Imports: []string{ + "github.com/golang/dep/internal/gps", + "sort", + }, + }, + }, + "varied/dotdotslash": { + Err: &LocalImportsError{ + Dir: j("varied_hidden/dotdotslash"), + ImportPath: "varied/dotdotslash", + LocalImports: []string{ + "../github.com/golang/dep/internal/gps", + }, + }, + }, + }, + }, + }, "invalid buildtag like comments should be ignored": { fileRoot: j("buildtag"), importRoot: "buildtag", @@ -1317,12 +1405,18 @@ func TestListPackages(t *testing.T) { "conflicting canonical comments": { fileRoot: j("canon_confl"), importRoot: "canon_confl", - out: PackageTree{}, - err: &ConflictingImportComments{ - ImportPath: "canon_confl", - ConflictingImportComments: []string{ - "vanity1", - "vanity2", + out: PackageTree{ + ImportRoot: "canon_confl", + Packages: map[string]PackageOrErr{ + "canon_confl": { + Err: &ConflictingImportComments{ + ImportPath: "canon_confl", + ConflictingImportComments: []string{ + "vanity1", + "vanity2", + }, + }, + }, }, }, }, @@ -1414,6 +1508,113 @@ func TestListPackages(t *testing.T) { } } +// Transform Table Test that operates solely on the varied_hidden fixture. +func TestTrimHiddenPackages(t *testing.T) { + base, err := ListPackages(filepath.Join(getTestdataRootDir(t), "src", "varied_hidden"), "varied") + if err != nil { + panic(err) + } + + table := map[string]struct { + main, tests bool // literal params to TrimHiddenPackages + ignore []string // transformed into IgnoredRuleset param to TrimHiddenPackages + trimmed []string // list of packages that should be trimmed in result PackageTree + }{ + // All of these implicitly verify that the varied/_never pkg is always + // trimmed, and that the varied/dotdotslash pkg is not trimmed even + // though it has errors. + "minimal trim": { + main: true, + tests: true, + }, + "ignore simple, lose testdata": { + main: true, + tests: true, + ignore: []string{"simple"}, + trimmed: []string{"simple", "simple/testdata"}, + }, + "no tests": { + main: true, + tests: false, + trimmed: []string{".onlyfromtests", "_secondorder"}, + }, + "ignore a reachable hidden": { + main: true, + tests: true, + ignore: []string{"_secondorder"}, + trimmed: []string{"_secondorder"}, + }, + "ignore a reachable hidden with another hidden solely reachable through it": { + main: true, + tests: true, + ignore: []string{".onlyfromtests"}, + trimmed: []string{".onlyfromtests", "_secondorder"}, + }, + "no main": { + main: false, + tests: true, + trimmed: []string{"", "_frommain"}, + }, + "no main or tests": { + main: false, + tests: false, + trimmed: []string{"", "_frommain", ".onlyfromtests", "_secondorder"}, + }, + "no main or tests and ignore simple": { + main: false, + tests: false, + ignore: []string{"simple"}, + trimmed: []string{"", "_frommain", ".onlyfromtests", "_secondorder", "simple", "simple/testdata"}, + }, + } + + for name, fix := range table { + t.Run(name, func(t *testing.T) { + want := base.Copy() + + var ig []string + for _, v := range fix.ignore { + ig = append(ig, path.Join("varied", v)) + } + got := base.TrimHiddenPackages(fix.main, fix.tests, NewIgnoredRuleset(ig)) + + for _, ip := range append(fix.trimmed, "_never") { + ip = path.Join("varied", ip) + if _, has := want.Packages[ip]; !has { + panic(fmt.Sprintf("bad input, %s does not exist in fixture ptree", ip)) + } + delete(want.Packages, ip) + } + + if !reflect.DeepEqual(want, got) { + if len(want.Packages) < 2 { + t.Errorf("Did not get expected PackageOrErrs:\n\t(GOT): %#v\n\t(WNT): %#v", got, want) + } else { + seen := make(map[string]bool) + for path, perr := range want.Packages { + seen[path] = true + if operr, exists := got.Packages[path]; !exists { + t.Errorf("Expected PackageOrErr for path %s was missing from output:\n\t%s", path, perr) + } else { + if !reflect.DeepEqual(perr, operr) { + t.Errorf("PkgOrErr for path %s was not as expected:\n\t(GOT): %#v\n\t(WNT): %#v", path, operr, perr) + } + } + } + + for path, operr := range got.Packages { + if seen[path] { + continue + } + + t.Errorf("Got PackageOrErr for path %s, but none was expected:\n\t%s", path, operr) + } + } + } + }) + } +} + // Test that ListPackages skips directories for which it lacks permissions to // enter and files it lacks permissions to read. func TestListPackagesNoPerms(t *testing.T) { @@ -1424,7 +1625,7 @@ func TestListPackagesNoPerms(t *testing.T) { // It's not a big deal, though, because the os.IsPermission() call we // use in the real code is effectively what's being tested here, and // that's designed to be cross-platform. So, if the unix tests pass, we - // have every reason to believe windows tests would to, if the situation + // have every reason to believe windows tests would too, if the situation // arises. t.Skip() } @@ -2035,199 +2236,37 @@ func getTestdataRootDir(t *testing.T) string { return filepath.Join(cwd, "..", "_testdata") } -func TestIgnoredRuleset(t *testing.T) { - type tfixm []struct { - path string - wild bool +// Canary regression test to make sure that if PackageTree ever gains new +// fields, we update the Copy method accordingly. +func TestCanaryPackageTreeCopy(t *testing.T) { + ptreeFields := []string{ + "ImportRoot", + "Packages", } - cases := []struct { - name string - inputs []string - wantInTree tfixm - wantEmptyTree bool - shouldIgnore []string - shouldNotIgnore []string - }{ - { - name: "only skip global ignore", - inputs: []string{"*"}, - wantEmptyTree: true, - }, - { - name: "ignores without ignore suffix", - inputs: []string{ - "x/y/z", - "*a/b/c", - "gophers", - }, - wantInTree: tfixm{ - {path: "x/y/z", wild: false}, - {path: "*a/b/c", wild: false}, - {path: "gophers", wild: false}, - }, - shouldIgnore: []string{ - "x/y/z", - "gophers", - }, - shouldNotIgnore: []string{ - "x/y/z/q", - "x/y", - "gopher", - "gopherss", - }, - }, - { - name: "ignores with ignore suffix", - inputs: []string{ - "x/y/z*", - "a/b/c", - "gophers", - }, - wantInTree: tfixm{ - {path: "x/y/z", wild: true}, - {path: "a/b/c", wild: false}, - {path: "gophers", wild: false}, - }, - shouldIgnore: []string{ - "x/y/z", - "x/y/zz", - "x/y/z/", - "x/y/z/q", - }, - shouldNotIgnore: []string{ - "x/y", - "gopher", - }, - }, - { - name: "global ignore with other strings", - inputs: []string{ - "*", - "gophers*", - "x/y/z*", - "a/b/c", - }, - wantInTree: tfixm{ - {path: "x/y/z", wild: true}, - {path: "a/b/c", wild: false}, - {path: "gophers", wild: true}, - }, - shouldIgnore: []string{ - "x/y/z", - "x/y/z/", - "x/y/z/q", - "gophers", - "gopherss", - "gophers/foo", - }, - shouldNotIgnore: []string{ - "x/y", - "gopher", - }, - }, - { - name: "ineffectual ignore with wildcard", - inputs: []string{ - "a/b*", - "a/b/c*", - "a/b/x/y", - "a/c*", - }, - wantInTree: tfixm{ - {path: "a/c", wild: true}, - {path: "a/b", wild: true}, - }, - shouldIgnore: []string{ - "a/cb", - }, - shouldNotIgnore: []string{ - "a/", - "a/d", - }, - }, - { - name: "same path with and without wildcard", - inputs: []string{ - "a/b*", - "a/b", - }, - wantInTree: tfixm{ - {path: "a/b", wild: true}, - }, - shouldIgnore: []string{ - "a/b", - "a/bb", - }, - shouldNotIgnore: []string{ - "a/", - "a/d", - }, - }, - { - name: "empty paths", - inputs: []string{ - "", - "a/b*", - }, - wantInTree: tfixm{ - {path: "a/b", wild: true}, - }, - shouldNotIgnore: []string{ - "", - }, - }, - { - name: "single wildcard", - inputs: []string{ - "a/b*", - }, - wantInTree: tfixm{ - {path: "a/b", wild: true}, - }, - shouldIgnore: []string{ - "a/b/c", - }, - }, + packageFields := []string{ + "Name", + "ImportPath", + "CommentPath", + "Imports", + "TestImports", } - for _, c := range cases { - igm := NewIgnoredRuleset(c.inputs) - f := func(t *testing.T) { - - if c.wantEmptyTree { - if igm.Len() != 0 { - t.Fatalf("wanted empty tree, but had %v elements", igm.Len()) - } - } + fieldNames := func(typ reflect.Type) []string { + var names []string + for i := 0; i < typ.NumField(); i++ { + names = append(names, typ.Field(i).Name) + } + return names + } - // Check if the wildcard suffix ignores are in the tree. - for _, p := range c.wantInTree { - wildi, has := igm.t.Get(p.path) - if !has { - t.Fatalf("expected %q to be in the tree", p.path) - } else if wildi.(bool) != p.wild { - if p.wild { - t.Fatalf("expected %q to be a wildcard matcher, but it was not", p.path) - } else { - t.Fatalf("expected %q not to be a wildcard matcher, but it was", p.path) - } - } - } + ptreeRefl := fieldNames(reflect.TypeOf(PackageTree{})) + packageRefl := fieldNames(reflect.TypeOf(Package{})) - for _, p := range c.shouldIgnore { - if !igm.IsIgnored(p) { - t.Fatalf("%q should be ignored, but it was not", p) - } - } - for _, p := range c.shouldNotIgnore { - if igm.IsIgnored(p) { - t.Fatalf("%q should not be ignored, but it was", p) - } - } - } - t.Run(c.name, f) + if !reflect.DeepEqual(ptreeFields, ptreeRefl) { + t.Errorf("PackageTree.Copy is designed to work with an exact set of fields in the PackageTree struct - make sure it (and this test) have been updated!\n\t(GOT):%s\n\t(WNT):%s", ptreeFields, ptreeRefl) + } - igm = NewIgnoredRuleset(igm.ToSlice()) - t.Run(c.name+"/inandout", f) + if !reflect.DeepEqual(packageFields, packageRefl) { + t.Errorf("PackageTree.Copy is designed to work with an exact set of fields in the Package struct - make sure it (and this test) have been updated!\n\t(GOT):%s\n\t(WNT):%s", packageFields, packageRefl) } } diff --git a/project.go b/project.go index 9749a723bf..4dd8ecda77 100644 --- a/project.go +++ b/project.go @@ -11,6 +11,7 @@ import ( "github.com/golang/dep/internal/fs" "github.com/golang/dep/internal/gps" + "github.com/golang/dep/internal/gps/pkgtree" "github.com/pkg/errors" ) @@ -133,6 +134,23 @@ func (p *Project) MakeParams() gps.SolveParameters { return params } +// ParseRootPackageTree analyzes the root project's disk contents to create a +// PackageTree, trimming out packages that are not relevant for root projects +// along the way. +func (p *Project) ParseRootPackageTree() (pkgtree.PackageTree, error) { + ptree, err := pkgtree.ListPackages(p.ResolvedAbsRoot, string(p.ImportRoot)) + if err != nil { + return pkgtree.PackageTree{}, errors.Wrap(err, "analysis of current project's packages failed") + } + // We don't care about (unreachable) hidden packages for the root project, + // so drop all of those. + var ig *pkgtree.IgnoredRuleset + if p.Manifest != nil { + ig = p.Manifest.IgnoredPackages() + } + return ptree.TrimHiddenPackages(true, true, ig), nil +} + // BackupVendor looks for existing vendor directory and if it's not empty, // creates a backup of it to a new directory with the provided suffix. func BackupVendor(vpath, suffix string) (string, error) {