Merge branch 'master' into feat/resolve-test-imports
diff --git a/action/create.go b/action/create.go index 4d1c0bb..398b8b8 100644 --- a/action/create.go +++ b/action/create.go
@@ -111,12 +111,13 @@ h := &dependency.DefaultMissingPackageHandler{Missing: []string{}, Gopath: []string{}} r.Handler = h - sortable, err := r.ResolveLocal(false) + sortable, testSortable, err := r.ResolveLocal(false) if err != nil { msg.Die("Error resolving local dependencies: %s", err) } sort.Strings(sortable) + sort.Strings(testSortable) vpath := r.VendorDir if !strings.HasSuffix(vpath, "/") { @@ -127,7 +128,7 @@ n := strings.TrimPrefix(pa, vpath) root, subpkg := util.NormalizeName(n) - if !config.HasDependency(root) && root != config.Name { + if !config.Imports.Has(root) && root != config.Name { msg.Info("--> Found reference to %s\n", n) d := &cfg.Dependency{ Name: root, @@ -136,7 +137,7 @@ d.Subpackages = []string{subpkg} } config.Imports = append(config.Imports, d) - } else if config.HasDependency(root) { + } else if config.Imports.Has(root) { if len(subpkg) > 0 { subpkg = strings.TrimPrefix(subpkg, "/") d := config.Imports.Get(root) @@ -148,6 +149,33 @@ } } + for _, pa := range testSortable { + n := strings.TrimPrefix(pa, vpath) + root, subpkg := util.NormalizeName(n) + + if config.Imports.Has(root) && root != config.Name { + msg.Debug("--> Found test reference to %s already listed as an import", n) + } else if !config.DevImports.Has(root) && root != config.Name { + msg.Info("--> Found test reference to %s", n) + d := &cfg.Dependency{ + Name: root, + } + if len(subpkg) > 0 { + d.Subpackages = []string{subpkg} + } + config.DevImports = append(config.DevImports, d) + } else if config.DevImports.Has(root) { + if len(subpkg) > 0 { + subpkg = strings.TrimPrefix(subpkg, "/") + d := config.DevImports.Get(root) + if !d.HasSubpackage(subpkg) { + msg.Info("--> Adding test sub-package %s to %s\n", subpkg, root) + d.Subpackages = append(d.Subpackages, subpkg) + } + } + } + } + if len(config.Imports) == importLen && importLen != 0 { msg.Info("--> Code scanning found no additional imports") }
diff --git a/action/list.go b/action/list.go index 8ff6c69..f4b3379 100644 --- a/action/list.go +++ b/action/list.go
@@ -28,7 +28,7 @@ h := &dependency.DefaultMissingPackageHandler{Missing: []string{}, Gopath: []string{}} r.Handler = h - localPkgs, err := r.ResolveLocal(deep) + localPkgs, _, err := r.ResolveLocal(deep) if err != nil { msg.Die("Error listing dependencies: %s", err) }
diff --git a/cfg/config.go b/cfg/config.go index afeed9a..58e3f07 100644 --- a/cfg/config.go +++ b/cfg/config.go
@@ -51,7 +51,7 @@ // DevImports contains the test or other development imports for a project. // See the Dependency type for more details on how this is recorded. - DevImports Dependencies `yaml:"devimport,omitempty"` + DevImports Dependencies `yaml:"testImport,omitempty"` } // A transitive representation of a dependency for importing and exporting to yaml. @@ -64,7 +64,7 @@ Ignore []string `yaml:"ignore,omitempty"` Exclude []string `yaml:"excludeDirs,omitempty"` Imports Dependencies `yaml:"import"` - DevImports Dependencies `yaml:"devimport,omitempty"` + DevImports Dependencies `yaml:"testImport,omitempty"` } // ConfigFromYaml returns an instance of Config from YAML @@ -298,6 +298,16 @@ return nil } +// Has checks if a dependency is on a list of dependencies such as import or devimport +func (d Dependencies) Has(name string) bool { + for _, dep := range d { + if dep.Name == name { + return true + } + } + return false +} + // Clone performs a deep clone of Dependencies func (d Dependencies) Clone() Dependencies { n := make(Dependencies, 0, len(d))
diff --git a/cfg/lock.go b/cfg/lock.go index 517d3ba..8b4d252 100644 --- a/cfg/lock.go +++ b/cfg/lock.go
@@ -15,7 +15,7 @@ Hash string `yaml:"hash"` Updated time.Time `yaml:"updated"` Imports Locks `yaml:"imports"` - DevImports Locks `yaml:"devImports"` + DevImports Locks `yaml:"testImports"` } // LockfileFromYaml returns an instance of Lockfile from YAML
diff --git a/dependency/resolver.go b/dependency/resolver.go index 300ca4d..d2b90cf 100644 --- a/dependency/resolver.go +++ b/dependency/resolver.go
@@ -140,7 +140,8 @@ ResolveAllFiles bool // Items already in the queue. - alreadyQ map[string]bool + alreadyQ map[string]bool + talreadyQ map[string]bool // Attempts to scan that had unrecoverable error. hadError map[string]bool @@ -222,7 +223,7 @@ if r.ResolveAllFiles { return r.resolveList(l) } - return r.resolveImports(l) + return r.resolveImports(l, false) } // dirHasPrefix tests whether the directory dir begins with prefix. @@ -240,12 +241,14 @@ // If the deep flag is set to true, this will then resolve all of the dependencies // of the dependencies it has found. If not, it will return just the packages that // the base project relies upon. -func (r *Resolver) ResolveLocal(deep bool) ([]string, error) { +func (r *Resolver) ResolveLocal(deep bool) ([]string, []string, error) { // We build a list of local source to walk, then send this list // to resolveList. msg.Debug("Resolving local dependencies") l := list.New() + tl := list.New() alreadySeen := map[string]bool{} + talreadySeen := map[string]bool{} err := filepath.Walk(r.basedir, func(path string, fi os.FileInfo, err error) error { if err != nil && err != filepath.SkipDir { return err @@ -266,6 +269,7 @@ // Scan for dependencies, and anything that's not part of the local // package gets added to the scan list. var imps []string + var testImps []string p, err := r.BuildContext.ImportDir(path, 0) if err != nil { if strings.HasPrefix(err.Error(), "no buildable Go source") { @@ -275,7 +279,7 @@ // declared. This is often because of an example with a package // or main but +build ignore as a build tag. In that case we // try to brute force the packages with a slower scan. - imps, err = IterativeScan(path) + imps, testImps, err = IterativeScan(path) if err != nil { return err } @@ -284,6 +288,7 @@ } } else { imps = p.Imports + testImps = p.TestImports } // We are only looking for dependencies in vendor. No root, cgo, etc. @@ -311,20 +316,46 @@ } } + for _, imp := range testImps { + if talreadySeen[imp] { + continue + } + talreadySeen[imp] = true + info := r.FindPkg(imp) + switch info.Loc { + case LocUnknown, LocVendor: + tl.PushBack(filepath.Join(r.VendorDir, filepath.FromSlash(imp))) // Do we need a path on this? + case LocGopath: + if !dirHasPrefix(info.Path, r.basedir) { + // FIXME: This is a package outside of the project we're + // scanning. It should really be on vendor. But we don't + // want it to reference GOPATH. We want it to be detected + // and moved. + tl.PushBack(filepath.Join(r.VendorDir, filepath.FromSlash(imp))) + } + case LocRelative: + if strings.HasPrefix(imp, "./"+gpath.VendorDir) { + msg.Warn("Go package resolving will resolve %s without the ./%s/ prefix", imp, gpath.VendorDir) + } + } + } + return nil }) if err != nil { msg.Err("Failed to build an initial list of packages to scan: %s", err) - return []string{}, err + return []string{}, []string{}, err } - if deep { - if r.ResolveAllFiles { - return r.resolveList(l) - } - return r.resolveImports(l) - } + // if deep { + // if r.ResolveAllFiles { + // re, err := r.resolveList(l) + // return re, []string{}, err + // } + // re, err := r.resolveImports(l, false) + // return re, []string{}, err + // } // If we're not doing a deep scan, we just convert the list into an // array and return. @@ -332,7 +363,12 @@ for e := l.Front(); e != nil; e = e.Next() { res = append(res, e.Value.(string)) } - return res, nil + tres := make([]string, 0, l.Len()) + for e := tl.Front(); e != nil; e = e.Next() { + tres = append(tres, e.Value.(string)) + } + + return res, tres, nil } // ResolveAll takes a list of packages and returns an inclusive list of all @@ -355,7 +391,7 @@ queue = list.New() } - loc, err := r.ResolveLocal(false) + loc, _, err := r.ResolveLocal(false) if err != nil { return []string{}, err } @@ -367,7 +403,7 @@ if r.ResolveAllFiles { return r.resolveList(queue) } - return r.resolveImports(queue) + return r.resolveImports(queue, false) } // stripv strips the vendor/ prefix from vendored packages. @@ -392,16 +428,25 @@ // // The resolver's handler is used in the cases where a package cannot be // located. -func (r *Resolver) resolveImports(queue *list.List) ([]string, error) { +func (r *Resolver) resolveImports(queue *list.List, testDeps bool) ([]string, error) { msg.Debug("Resolving import path") + if testDeps { + msg.Debug("Resolving test dependencies") + } for e := queue.Front(); e != nil; e = e.Next() { vdep := e.Value.(string) dep := r.stripv(vdep) // Check if marked in the Q and then explicitly mark it. We want to know // if it had previously been marked and ensure it for the future. - _, foundQ := r.alreadyQ[dep] - r.alreadyQ[dep] = true + var foundQ bool + if testDeps { + _, foundQ = r.talreadyQ[dep] + r.talreadyQ[dep] = true + } else { + _, foundQ = r.alreadyQ[dep] + r.alreadyQ[dep] = true + } // If we've already encountered an error processing this dependency // skip it. @@ -427,7 +472,12 @@ // or main but +build ignore as a build tag. In that case we // try to brute force the packages with a slower scan. msg.Debug("Using Iterative Scanning for %s", dep) - imps, err = IterativeScan(vdep) + if testDeps { + _, imps, err = IterativeScan(vdep) + } else { + imps, _, err = IterativeScan(vdep) + } + if err != nil { msg.Err("Iterative scanning error %s: %s", dep, err) continue @@ -446,7 +496,11 @@ // If the location doesn't exist try to fetch it. if ok, err2 := r.Handler.NotFound(dep); ok { - r.alreadyQ[dep] = true + if testDeps { + r.talreadyQ[dep] = true + } else { + r.alreadyQ[dep] = true + } // By adding to the queue it will get reprocessed now that // it exists. @@ -467,7 +521,12 @@ } continue } else { - imps = pkg.Imports + if testDeps { + imps = pkg.TestImports + } else { + imps = pkg.Imports + } + } // Range over all of the identified imports and see which ones we @@ -486,7 +545,11 @@ msg.Debug("In vendor: %s", imp) if _, ok := r.alreadyQ[imp]; !ok { msg.Debug("Marking %s to be scanned.", imp) - r.alreadyQ[imp] = true + if testDeps { + r.talreadyQ[dep] = true + } else { + r.alreadyQ[dep] = true + } queue.PushBack(r.vpath(imp)) if err := r.Handler.InVendor(imp); err == nil { r.VersionHandler.SetVersion(imp) @@ -497,7 +560,11 @@ case LocUnknown: msg.Debug("Missing %s. Trying to resolve.", imp) if ok, err := r.Handler.NotFound(imp); ok { - r.alreadyQ[imp] = true + if testDeps { + r.talreadyQ[dep] = true + } else { + r.alreadyQ[dep] = true + } queue.PushBack(r.vpath(imp)) r.VersionHandler.SetVersion(imp) } else if err != nil { @@ -512,7 +579,11 @@ if _, ok := r.alreadyQ[imp]; !ok { // Only scan it if it gets moved into vendor/ if ok, _ := r.Handler.OnGopath(imp); ok { - r.alreadyQ[imp] = true + if testDeps { + r.talreadyQ[dep] = true + } else { + r.alreadyQ[dep] = true + } queue.PushBack(r.vpath(imp)) r.VersionHandler.SetVersion(imp) } @@ -695,7 +766,7 @@ // declared. This is often because of an example with a package // or main but +build ignore as a build tag. In that case we // try to brute force the packages with a slower scan. - imps, err = IterativeScan(pkg) + imps, _, err = IterativeScan(pkg) if err != nil { return []string{}, err }
diff --git a/dependency/resolver_test.go b/dependency/resolver_test.go index bb455c2..f62b4f5 100644 --- a/dependency/resolver_test.go +++ b/dependency/resolver_test.go
@@ -15,7 +15,7 @@ t.Fatal(err) } - l, err := r.ResolveLocal(false) + l, _, err := r.ResolveLocal(false) if err != nil { t.Fatalf("Failed to resolve: %s", err) } @@ -47,7 +47,7 @@ t.Fatal(err) } - l, err := r.ResolveLocal(true) + l, _, err := r.ResolveLocal(true) if err != nil { t.Fatalf("Failed to resolve: %s", err) }
diff --git a/dependency/scan.go b/dependency/scan.go index c1bad5c..71f4865 100644 --- a/dependency/scan.go +++ b/dependency/scan.go
@@ -35,7 +35,7 @@ // Note, there are cases where multiple packages are in the same directory. This // usually happens with an example that has a main package and a +build tag // of ignore. This is a bit of a hack. It causes UseAllFiles to have errors. -func IterativeScan(path string) ([]string, error) { +func IterativeScan(path string) ([]string, []string, error) { // TODO(mattfarina): Add support for release tags. @@ -44,6 +44,7 @@ tgs = append(tgs, "") var pkgs []string + var testPkgs []string for _, tt := range tgs { // split the tag combination to look at permutations. @@ -84,7 +85,7 @@ b, err := util.GetBuildContext() if err != nil { - return []string{}, err + return []string{}, []string{}, err } // Make sure use all files is off @@ -110,7 +111,7 @@ continue } else if err != nil { msg.Debug("Problem parsing package at %s for %s %s", path, ops, arch) - return []string{}, err + return []string{}, []string{}, err } for _, dep := range pk.Imports { @@ -124,9 +125,21 @@ pkgs = append(pkgs, dep) } } + + for _, dep := range pk.TestImports { + found := false + for _, p := range pkgs { + if p == dep { + found = true + } + } + if !found { + testPkgs = append(testPkgs, dep) + } + } } - return pkgs, nil + return pkgs, testPkgs, nil } func readBuildTags(p string) ([]string, error) {
diff --git a/docs/glide.yaml.md b/docs/glide.yaml.md index 2943853..f0173ac 100644 --- a/docs/glide.yaml.md +++ b/docs/glide.yaml.md
@@ -44,4 +44,4 @@ - `subpackages`: A record of packages being used within a repository. This does not include all packages within a repository but rather those being used. - `os`: A list of operating systems used for filtering. If set it will compare the current runtime OS to the one specified and only fetch the dependency if there is a match. If not set filtering is skipped. The names are the same used in build flags and `GOOS` environment variable. - `arch`: A list of architectures used for filtering. If set it will compare the current runtime architecture to the one specified and only fetch the dependency if there is a match. If not set filtering is skipped. The names are the same used in build flags and `GOARCH` environment variable. -- `devImport`: A list of development packages. Each package has the same details as those listed under import. +- `testImport`: A list of packages used in tests that are not already listed in `import`. Each package has the same details as those listed under import.
diff --git a/tree/tree.go b/tree/tree.go index 5b19336..0dc3df7 100644 --- a/tree/tree.go +++ b/tree/tree.go
@@ -59,7 +59,7 @@ // declared. This is often because of an example with a package // or main but +build ignore as a build tag. In that case we // try to brute force the packages with a slower scan. - imps, err = dependency.IterativeScan(path) + imps, _, err = dependency.IterativeScan(path) if err != nil { msg.Err("Error walking dependencies for %s: %s", path, err) return err