Merge pull request #1534 from sdboyer/ineffectuals

Warn on ineffectual constraints

Author: sdboyer

CHANGELOG.md

@@ -29,6 +29,7 @@ IMPROVEMENTS:
 * Add constraint for locked projects in `dep status`. ([#962](https://github.com/golang/dep/pull/962)
 * Make external config importers error tolerant. ([#1315](https://github.com/golang/dep/pull/1315))
 * Show LATEST and VERSION as the same type in status. ([#1515](https://github.com/golang/dep/pull/1515)
+* Warn when [[constraint]] rules that will have no effect. ([#1534](https://github.com/golang/dep/pull/1534))
 
 # v0.3.2
 

cmd/dep/ensure.go

@@ -197,6 +197,20 @@ func (cmd *ensureCommand) Run(ctx *dep.Ctx, args []string) error {
 			ctx.Out.Println(err)
 		}
 	}
+	if ineffs := p.FindIneffectualConstraints(sm); len(ineffs) > 0 {
+		ctx.Err.Printf("Warning: the following project(s) have [[constraint]] stanzas in %s:\n\n", dep.ManifestName)
+		for _, ineff := range ineffs {
+			ctx.Err.Println("  ✗ ", ineff)
+		}
+		// TODO(sdboyer) lazy wording, it does not mention ignores at all
+		ctx.Err.Printf("\nHowever, these projects are not direct dependencies of the current project:\n")
+		ctx.Err.Printf("they are not imported in any .go files, nor are they in the 'required' list in\n")
+		ctx.Err.Printf("%s. Dep only applies [[constraint]] rules to direct dependencies, so\n", dep.ManifestName)
+		ctx.Err.Printf("these rules will have no effect.\n\n")
+		ctx.Err.Printf("Either import/require packages from these projects so that they become direct\n")
+		ctx.Err.Printf("dependencies, or convert each [[constraint]] to an [[override]] to enforce rules\n")
+		ctx.Err.Printf("on these projects, if they happen to be transitive dependencies,\n\n")
+	}
 
 	if cmd.add {
 		return cmd.runAdd(ctx, args, p, sm, params)
@@ -515,7 +529,7 @@ func (cmd *ensureCommand) runAdd(ctx *dep.Ctx, args []string, p *dep.Project, sm
 			}
 
 			inManifest := p.Manifest.HasConstraintsOn(pc.Ident.ProjectRoot)
-			inImports := exrmap[pc.Ident.ProjectRoot]
+			inImports := exmap[string(pc.Ident.ProjectRoot)]
 			if inManifest && inImports {
 				errCh <- errors.Errorf("nothing to -add, %s is already in %s and the project's direct imports or required list", pc.Ident.ProjectRoot, dep.ManifestName)
 				return

cmd/dep/failures.go

@@ -0,0 +1,23 @@
+// Copyright 2018 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 (
+	"context"
+
+	"github.com/golang/dep/gps"
+	"github.com/pkg/errors"
+)
+
+// TODO solve failures can be really creative - we need to be similarly creative
+// in handling them and informing the user appropriately
+func handleAllTheFailuresOfTheWorld(err error) error {
+	switch errors.Cause(err) {
+	case context.Canceled, context.DeadlineExceeded, gps.ErrSourceManagerIsReleased:
+		return nil
+	}
+
+	return errors.Wrap(err, "Solving failure")
+}

cmd/dep/gopath_scanner.go

@@ -24,15 +24,15 @@ import (
 // It uses its results to fill-in any missing details left by the rootAnalyzer.
 type gopathScanner struct {
 	ctx        *dep.Ctx
-	directDeps map[string]bool
+	directDeps map[gps.ProjectRoot]bool
 	sm         gps.SourceManager
 
 	pd    projectData
 	origM *dep.Manifest
 	origL *dep.Lock
 }
 
-func newGopathScanner(ctx *dep.Ctx, directDeps map[string]bool, sm gps.SourceManager) *gopathScanner {
+func newGopathScanner(ctx *dep.Ctx, directDeps map[gps.ProjectRoot]bool, sm gps.SourceManager) *gopathScanner {
 	return &gopathScanner{
 		ctx:        ctx,
 		directDeps: directDeps,
@@ -113,7 +113,7 @@ func (g *gopathScanner) overlay(rootM *dep.Manifest, rootL *dep.Lock) {
 		rootL.P = append(rootL.P, lp)
 		lockedProjects[pkg] = true
 
-		if _, isDirect := g.directDeps[string(pkg)]; !isDirect {
+		if _, isDirect := g.directDeps[pkg]; !isDirect {
 			f := fb.NewLockedProjectFeedback(lp, fb.DepTypeTransitive)
 			f.LogFeedback(g.ctx.Err)
 		}
@@ -220,7 +220,10 @@ func (g *gopathScanner) scanGopathForDependencies() (projectData, error) {
 		return projectData{}, nil
 	}
 
-	for ip := range g.directDeps {
+	for ippr := range g.directDeps {
+		// TODO(sdboyer) these are not import paths by this point, they've
+		// already been worked down to project roots.
+		ip := string(ippr)
 		pr, err := g.sm.DeduceProjectRoot(ip)
 		if err != nil {
 			return projectData{}, errors.Wrap(err, "sm.DeduceProjectRoot")

cmd/dep/init.go

@@ -15,13 +15,11 @@ import (
 
 	"github.com/golang/dep"
 	"github.com/golang/dep/gps"
-	"github.com/golang/dep/gps/paths"
-	"github.com/golang/dep/gps/pkgtree"
 	"github.com/golang/dep/internal/fs"
 	"github.com/pkg/errors"
 )
 
-const initShortHelp = `Initialize a new project with manifest and lock files`
+const initShortHelp = `Set up a new Go project, or migrate an existing one`
 const initLongHelp = `
 Initialize the project at filepath root by parsing its dependencies, writing
 manifest and lock files, and vendoring the dependencies. If root isn't
@@ -89,43 +87,10 @@ func (cmd *initCommand) Run(ctx *dep.Ctx, args []string) error {
 		}
 	}
 
-	var err error
-	p := new(dep.Project)
-	if err = p.SetRoot(root); err != nil {
-		return errors.Wrapf(err, "init failed: unable to set the root project to %s", root)
-	}
-
-	ctx.GOPATH, err = ctx.DetectProjectGOPATH(p)
-	if err != nil {
-		return errors.Wrapf(err, "init failed: unable to detect the containing GOPATH")
-	}
-
-	mf := filepath.Join(root, dep.ManifestName)
-	lf := filepath.Join(root, dep.LockName)
-	vpath := filepath.Join(root, "vendor")
-
-	mok, err := fs.IsRegular(mf)
+	p, err := cmd.establishProjectAt(root, ctx)
 	if err != nil {
-		return errors.Wrapf(err, "init failed: unable to check for an existing manifest at %s", mf)
+		return err
 	}
-	if mok {
-		return errors.Errorf("init aborted: manifest already exists at %s", mf)
-	}
-	// Manifest file does not exist.
-
-	lok, err := fs.IsRegular(lf)
-	if err != nil {
-		return errors.Wrapf(err, "init failed: unable to check for an existing lock at %s", lf)
-	}
-	if lok {
-		return errors.Errorf("invalid aborted: lock already exists at %s", lf)
-	}
-
-	ip, err := ctx.ImportForAbs(root)
-	if err != nil {
-		return errors.Wrapf(err, "init failed: unable to determine the import path for the root project %s", root)
-	}
-	p.ImportRoot = gps.ProjectRoot(ip)
 
 	sm, err := ctx.SourceManager()
 	if err != nil {
@@ -137,12 +102,13 @@ func (cmd *initCommand) Run(ctx *dep.Ctx, args []string) error {
 	if ctx.Verbose {
 		ctx.Out.Println("Getting direct dependencies...")
 	}
-	pkgT, directDeps, err := getDirectDependencies(sm, p)
+
+	ptree, directDeps, err := p.GetDirectDependencyNames(sm)
 	if err != nil {
 		return errors.Wrap(err, "init failed: unable to determine direct dependencies")
 	}
 	if ctx.Verbose {
-		ctx.Out.Printf("Checked %d directories for packages.\nFound %d direct dependencies.\n", len(pkgT.Packages), len(directDeps))
+		ctx.Out.Printf("Checked %d directories for packages.\nFound %d direct dependencies.\n", len(ptree.Packages), len(directDeps))
 	}
 
 	// Initialize with imported data, then fill in the gaps using the GOPATH
@@ -165,7 +131,7 @@ func (cmd *initCommand) Run(ctx *dep.Ctx, args []string) error {
 
 	params := gps.SolveParameters{
 		RootDir:         root,
-		RootPackageTree: pkgT,
+		RootPackageTree: ptree,
 		Manifest:        p.Manifest,
 		Lock:            p.Lock,
 		ProjectAnalyzer: rootAnalyzer,
@@ -203,7 +169,7 @@ func (cmd *initCommand) Run(ctx *dep.Ctx, args []string) error {
 	p.Lock.SolveMeta.InputsDigest = s.HashInputs()
 
 	// Pass timestamp (yyyyMMddHHmmss format) as suffix to backup name.
-	vendorbak, err := dep.BackupVendor(vpath, time.Now().Format("20060102150405"))
+	vendorbak, err := dep.BackupVendor(filepath.Join(root, "vendor"), time.Now().Format("20060102150405"))
 	if err != nil {
 		return errors.Wrap(err, "init failed: first backup vendor/, delete it, and then retry the previous command: failed to backup existing vendor directory")
 	}
@@ -227,32 +193,50 @@ func (cmd *initCommand) Run(ctx *dep.Ctx, args []string) error {
 	return nil
 }
 
-func getDirectDependencies(sm gps.SourceManager, p *dep.Project) (pkgtree.PackageTree, map[string]bool, error) {
-	pkgT, err := p.ParseRootPackageTree()
+// establishProjectAt attempts to set up the provided path as the root for the
+// project to be created.
+//
+// It checks for being within a GOPATH, that there is no pre-existing manifest
+// and lock, and that we can successfully infer the root import path from
+// GOPATH.
+//
+// If successful, it returns a dep.Project, ready for further use.
+func (cmd *initCommand) establishProjectAt(root string, ctx *dep.Ctx) (*dep.Project, error) {
+	var err error
+	p := new(dep.Project)
+	if err = p.SetRoot(root); err != nil {
+		return nil, errors.Wrapf(err, "init failed: unable to set the root project to %s", root)
+	}
+
+	ctx.GOPATH, err = ctx.DetectProjectGOPATH(p)
 	if err != nil {
-		return pkgtree.PackageTree{}, nil, err
+		return nil, errors.Wrapf(err, "init failed: unable to detect the containing GOPATH")
 	}
 
-	directDeps := map[string]bool{}
-	rm, _ := pkgT.ToReachMap(true, true, false, nil)
-	for _, ip := range rm.FlattenFn(paths.IsStandardImportPath) {
-		pr, err := sm.DeduceProjectRoot(ip)
-		if err != nil {
-			return pkgtree.PackageTree{}, nil, err
-		}
-		directDeps[string(pr)] = true
+	mf := filepath.Join(root, dep.ManifestName)
+	lf := filepath.Join(root, dep.LockName)
+
+	mok, err := fs.IsRegular(mf)
+	if err != nil {
+		return nil, errors.Wrapf(err, "init failed: unable to check for an existing manifest at %s", mf)
+	}
+	if mok {
+		return nil, errors.Errorf("init aborted: manifest already exists at %s", mf)
 	}
 
-	return pkgT, directDeps, nil
-}
+	lok, err := fs.IsRegular(lf)
+	if err != nil {
+		return nil, errors.Wrapf(err, "init failed: unable to check for an existing lock at %s", lf)
+	}
+	if lok {
+		return nil, errors.Errorf("invalid aborted: lock already exists at %s", lf)
+	}
 
-// TODO solve failures can be really creative - we need to be similarly creative
-// in handling them and informing the user appropriately
-func handleAllTheFailuresOfTheWorld(err error) error {
-	switch errors.Cause(err) {
-	case context.Canceled, context.DeadlineExceeded, gps.ErrSourceManagerIsReleased:
-		return nil
+	ip, err := ctx.ImportForAbs(root)
+	if err != nil {
+		return nil, errors.Wrapf(err, "init failed: unable to determine the import path for the root project %s", root)
 	}
+	p.ImportRoot = gps.ProjectRoot(ip)
 
-	return errors.Wrap(err, "Solving failure")
+	return p, nil
 }

cmd/dep/init_test.go

@@ -26,10 +26,10 @@ type rootAnalyzer struct {
 	skipTools  bool
 	ctx        *dep.Ctx
 	sm         gps.SourceManager
-	directDeps map[string]bool
+	directDeps map[gps.ProjectRoot]bool
 }
 
-func newRootAnalyzer(skipTools bool, ctx *dep.Ctx, directDeps map[string]bool, sm gps.SourceManager) *rootAnalyzer {
+func newRootAnalyzer(skipTools bool, ctx *dep.Ctx, directDeps map[gps.ProjectRoot]bool, sm gps.SourceManager) *rootAnalyzer {
 	return &rootAnalyzer{
 		skipTools:  skipTools,
 		ctx:        ctx,
@@ -73,7 +73,7 @@ func (a *rootAnalyzer) cacheDeps(pr gps.ProjectRoot) error {
 		return nil
 	}
 
-	deps := make(chan string)
+	deps := make(chan gps.ProjectRoot)
 
 	for i := 0; i < concurrency; i++ {
 		g.Go(func() error {
@@ -132,7 +132,7 @@ func (a *rootAnalyzer) importManifestAndLock(dir string, pr gps.ProjectRoot, sup
 
 func (a *rootAnalyzer) removeTransitiveDependencies(m *dep.Manifest) {
 	for pr := range m.Constraints {
-		if _, isDirect := a.directDeps[string(pr)]; !isDirect {
+		if _, isDirect := a.directDeps[pr]; !isDirect {
 			delete(m.Constraints, pr)
 		}
 	}
@@ -172,7 +172,7 @@ func (a *rootAnalyzer) FinalizeRootManifestAndLock(m *dep.Manifest, l *dep.Lock,
 		var f *fb.ConstraintFeedback
 		pr := y.Ident().ProjectRoot
 		// New constraints: in new lock and dir dep but not in manifest
-		if _, ok := a.directDeps[string(pr)]; ok {
+		if _, ok := a.directDeps[pr]; ok {
 			if _, ok := m.Constraints[pr]; !ok {
 				pp := getProjectPropertiesFromVersion(y.Version())
 				if pp.Constraint != nil {

cmd/dep/root_analyzer.go

@@ -764,8 +764,9 @@ func collectConstraints(ctx *dep.Ctx, p *dep.Project, sm gps.SourceManager) (con
 	var mutex sync.Mutex
 	constraintCollection := make(constraintsCollection)
 
-	// Get direct deps of the root project.
-	_, directDeps, err := getDirectDependencies(sm, p)
+	// Collect the complete set of direct project dependencies, incorporating
+	// requireds and ignores appropriately.
+	_, directDeps, err := p.GetDirectDependencyNames(sm)
 	if err != nil {
 		// Return empty collection, not nil, if we fail here.
 		return constraintCollection, []error{errors.Wrap(err, "failed to get direct dependencies")}
@@ -805,7 +806,7 @@ func collectConstraints(ctx *dep.Ctx, p *dep.Project, sm gps.SourceManager) (con
 			// project and constraint values.
 			for pr, pp := range pc {
 				// Check if the project constraint is imported in the root project
-				if _, ok := directDeps[string(pr)]; !ok {
+				if _, ok := directDeps[pr]; !ok {
 					continue
 				}