Trim hidden packages from (root) PackageTrees

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:

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{}

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)
 				}
 

internal/gps/pkgtree/pkgtree.go

@@ -140,20 +140,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 +154,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{
@@ -634,6 +633,56 @@ func (t PackageTree) Copy() PackageTree {
 	return t2
 }
 
+// TrimHiddenPackages returns a new PackageTree where packages that are both
+// hidden and unreachable have been removed. Ignored packages are optionally
+// removed, as well.
+//
+// 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.
+//
+// "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, keepIgnored bool, ignore *IgnoredRuleset) PackageTree {
+	rm, _ := t.ToReachMap(main, tests, false, ignore)
+	t2 := t.Copy()
+
+	preserve := make(map[string]bool)
+	for pkg, ie := range rm {
+		if pkgFilter(pkg) && (keepIgnored || !ignore.IsIgnored(pkg)) {
+			preserve[pkg] = true
+			for _, in := range ie.Internal {
+				preserve[in] = true
+			}
+		}
+	}
+
+	for ip := range t.Packages {
+		if !preserve[ip] {
+			delete(t2.Packages, ip)
+		}
+	}
+
+	return t2
+}
+
 // wmToReach takes an internal "workmap" constructed by
 // PackageTree.ExternalReach(), transitively walks (via depth-first traversal)
 // all internal imports until they reach an external path or terminate, then

internal/gps/pkgtree/pkgtree_test.go

@@ -936,27 +936,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"),
@@ -1317,12 +1296,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",
+							},
+						},
+					},
 				},
 			},
 		},

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,19 @@ 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.
+	return ptree.TrimHiddenPackages(true, true, false, p.Manifest.IgnoredPackages()), 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) {