diff --git a/CHANGELOG.md b/CHANGELOG.md index b84258f1d5..dac5a2f56e 100644 --- a/CHANGELOG.md +++ b/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 diff --git a/cmd/dep/ensure.go b/cmd/dep/ensure.go index 3b0b1041ee..202ec35505 100644 --- a/cmd/dep/ensure.go +++ b/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 diff --git a/cmd/dep/failures.go b/cmd/dep/failures.go new file mode 100644 index 0000000000..c40ac8c785 --- /dev/null +++ b/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") +} diff --git a/cmd/dep/gopath_scanner.go b/cmd/dep/gopath_scanner.go index 5bf82ccddd..ed94d8d49f 100644 --- a/cmd/dep/gopath_scanner.go +++ b/cmd/dep/gopath_scanner.go @@ -24,7 +24,7 @@ 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 @@ -32,7 +32,7 @@ type gopathScanner struct { 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") diff --git a/cmd/dep/init.go b/cmd/dep/init.go index e09c1f20b0..e8c255c87d 100644 --- a/cmd/dep/init.go +++ b/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 } diff --git a/cmd/dep/init_test.go b/cmd/dep/init_test.go deleted file mode 100644 index 0d0aff23ba..0000000000 --- a/cmd/dep/init_test.go +++ /dev/null @@ -1,40 +0,0 @@ -// Copyright 2016 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 ( - "path/filepath" - "testing" - - "github.com/golang/dep" - "github.com/golang/dep/gps" - "github.com/golang/dep/internal/test" -) - -func TestGetDirectDependencies_ConsolidatesRootProjects(t *testing.T) { - h := test.NewHelper(t) - defer h.Cleanup() - - ctx := NewTestContext(h) - sm, err := ctx.SourceManager() - h.Must(err) - defer sm.Release() - - testprj := "directdepstest" - testdir := filepath.Join("src", testprj) - h.TempDir(testdir) - h.TempCopy(filepath.Join(testdir, "main.go"), "init/directdeps/main.go") - - testpath := h.Path(testdir) - prj := &dep.Project{AbsRoot: testpath, ResolvedAbsRoot: testpath, ImportRoot: gps.ProjectRoot(testprj)} - - _, dd, err := getDirectDependencies(sm, prj) - h.Must(err) - - wantpr := "github.com/carolynvs/deptest-subpkg" - if _, has := dd[wantpr]; !has { - t.Fatalf("Expected direct dependencies to contain %s, got %v", wantpr, dd) - } -} diff --git a/cmd/dep/root_analyzer.go b/cmd/dep/root_analyzer.go index 0dcf8683b3..76d410fe6e 100644 --- a/cmd/dep/root_analyzer.go +++ b/cmd/dep/root_analyzer.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 { diff --git a/cmd/dep/status.go b/cmd/dep/status.go index 1a24696cc6..c342b20f29 100644 --- a/cmd/dep/status.go +++ b/cmd/dep/status.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 } diff --git a/project.go b/project.go index 2da46dc497..d2677e8866 100644 --- a/project.go +++ b/project.go @@ -8,8 +8,10 @@ import ( "fmt" "os" "path/filepath" + "sort" "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" @@ -99,9 +101,10 @@ type Project struct { // If AbsRoot is not a symlink, then ResolvedAbsRoot should equal AbsRoot. ResolvedAbsRoot string // ImportRoot is the import path of the project's root directory. - ImportRoot gps.ProjectRoot - Manifest *Manifest - Lock *Lock // Optional + ImportRoot gps.ProjectRoot + Manifest *Manifest + Lock *Lock // Optional + RootPackageTree pkgtree.PackageTree } // SetRoot sets the project AbsRoot and ResolvedAbsRoot. If root is not a symlink, ResolvedAbsRoot will be set to root. @@ -137,18 +140,112 @@ func (p *Project) MakeParams() gps.SolveParameters { // 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. +// +// The resulting tree is cached internally at p.RootPackageTree. func (p *Project) ParseRootPackageTree() (pkgtree.PackageTree, error) { - ptree, err := pkgtree.ListPackages(p.ResolvedAbsRoot, string(p.ImportRoot)) + if p.RootPackageTree.Packages == nil { + 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() + } + p.RootPackageTree = ptree.TrimHiddenPackages(true, true, ig) + } + return p.RootPackageTree, nil +} + +// GetDirectDependencyNames returns the set of unique Project Roots that are the +// direct dependencies of this Project. +// +// A project is considered a direct dependency if at least one of packages in it +// is named in either this Project's required list, or if there is at least one +// non-ignored import statement from a non-ignored package in the current +// project's package tree. +// +// The returned map of Project Roots contains only boolean true values; this +// makes a "false" value always indicate an absent key, which makes conditional +// checks against the map more ergonomic. +// +// This function will correctly utilize ignores and requireds from an existing +// manifest, if one is present, but will also do the right thing without a +// manifest. +func (p *Project) GetDirectDependencyNames(sm gps.SourceManager) (pkgtree.PackageTree, map[gps.ProjectRoot]bool, error) { + ptree, err := p.ParseRootPackageTree() if err != nil { - return pkgtree.PackageTree{}, errors.Wrap(err, "analysis of current project's packages failed") + return pkgtree.PackageTree{}, nil, err } - // We don't care about (unreachable) hidden packages for the root project, - // so drop all of those. + var ig *pkgtree.IgnoredRuleset + var req map[string]bool if p.Manifest != nil { ig = p.Manifest.IgnoredPackages() + req = p.Manifest.RequiredPackages() + } + + rm, _ := ptree.ToReachMap(true, true, false, ig) + reach := rm.FlattenFn(paths.IsStandardImportPath) + + if len(req) > 0 { + // Make a map of imports that are both in the import path list and the + // required list to avoid duplication. + skip := make(map[string]bool, len(req)) + for _, r := range reach { + if req[r] { + skip[r] = true + } + } + + for r := range req { + if !skip[r] { + reach = append(reach, r) + } + } } - return ptree.TrimHiddenPackages(true, true, ig), nil + + directDeps := map[gps.ProjectRoot]bool{} + for _, ip := range reach { + pr, err := sm.DeduceProjectRoot(ip) + if err != nil { + return pkgtree.PackageTree{}, nil, err + } + directDeps[pr] = true + } + + return ptree, directDeps, nil +} + +// FindIneffectualConstraints looks for constraint rules expressed in the +// manifest that will have no effect during solving, as they are specified for +// projects that are not direct dependencies of the Project. +// +// "Direct dependency" here is as implemented by GetDirectDependencyNames(); +// it correctly incorporates all "ignored" and "required" rules. +func (p *Project) FindIneffectualConstraints(sm gps.SourceManager) []gps.ProjectRoot { + if p.Manifest == nil { + return nil + } + + _, dd, err := p.GetDirectDependencyNames(sm) + if err != nil { + return nil + } + + var ineff []gps.ProjectRoot + for pr := range p.Manifest.DependencyConstraints() { + if !dd[pr] { + ineff = append(ineff, pr) + } + } + + sort.Slice(ineff, func(i, j int) bool { + return ineff[i] < ineff[j] + }) + return ineff } // BackupVendor looks for existing vendor directory and if it's not empty,