Skip to content

Commit

Permalink
More resolution coverage of test deps
Browse files Browse the repository at this point in the history
  • Loading branch information
@mattfarina
mattfarina committed Jun 9, 2016
1 parent e426529 commit bed8a15
Show file tree
Hide file tree
Showing 4 changed files with 115 additions and 49 deletions.
3 changes: 3 additions & 0 deletions action/create.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -108,6 +108,9 @@ func guessDeps(base string, skipImport bool) *cfg.Config {
msg.Die("Error creating a dependency resolver: %s", err)
}

// When creating resolve the test dependencies as well as the application ones.
r.ResolveTest = true

h := &dependency.DefaultMissingPackageHandler{Missing: []string{}, Gopath: []string{}}
r.Handler = h

Expand Down
125 changes: 79 additions & 46 deletions dependency/resolver.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,7 @@ type VersionHandler interface {

// SetVersion sets the version for a package. An error is returned if there
// was a problem setting the version.
SetVersion(pkg string) error
SetVersion(pkg string, testDep bool) error
}

// DefaultVersionHandler is the default handler for setting the version.
Expand All @@ -113,7 +113,7 @@ func (d *DefaultVersionHandler) Process(pkg string) error {

// SetVersion here sends a message when a package is found noting that it
// did not set the version.
func (d *DefaultVersionHandler) SetVersion(pkg string) error {
func (d *DefaultVersionHandler) SetVersion(pkg string, testDep bool) error {
msg.Warn("Version not set for package %s", pkg)
return nil
}
Expand All @@ -139,6 +139,9 @@ type Resolver struct {
// import tree.
ResolveAllFiles bool

// ResolveTest sets if test dependencies should be resolved.
ResolveTest bool

// Items already in the queue.
alreadyQ map[string]bool
talreadyQ map[string]bool
Expand Down Expand Up @@ -221,7 +224,7 @@ func (r *Resolver) Resolve(pkg, basepath string) ([]string, error) {

// In this mode, walk the entire tree.
if r.ResolveAllFiles {
return r.resolveList(l)
return r.resolveList(l, false)
}
return r.resolveImports(l, false)
}
Expand Down Expand Up @@ -316,26 +319,28 @@ func (r *Resolver) ResolveLocal(deep bool) ([]string, []string, error) {
}
}

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)))
if r.ResolveTest {
for _, imp := range testImps {
if talreadySeen[imp] {
continue
}
case LocRelative:
if strings.HasPrefix(imp, "./"+gpath.VendorDir) {
msg.Warn("Go package resolving will resolve %s without the ./%s/ prefix", imp, gpath.VendorDir)
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)
}
}
}
}
Expand All @@ -348,14 +353,22 @@ func (r *Resolver) ResolveLocal(deep bool) ([]string, []string, error) {
return []string{}, []string{}, err
}

// 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 deep {
if r.ResolveAllFiles {
re, err := r.resolveList(l, false)
if err != nil {
return []string{}, []string{}, err
}
tre, err := r.resolveList(l, false)
return re, tre, err
}
re, err := r.resolveImports(l, false)
if err != nil {
return []string{}, []string{}, err
}
tre, err := r.resolveImports(tl, true)
return re, tre, err
}

// If we're not doing a deep scan, we just convert the list into an
// array and return.
Expand All @@ -364,8 +377,10 @@ func (r *Resolver) ResolveLocal(deep bool) ([]string, []string, error) {
res = append(res, e.Value.(string))
}
tres := make([]string, 0, l.Len())
for e := tl.Front(); e != nil; e = e.Next() {
tres = append(tres, e.Value.(string))
if r.ResolveTest {
for e := tl.Front(); e != nil; e = e.Next() {
tres = append(tres, e.Value.(string))
}
}

return res, tres, nil
Expand Down Expand Up @@ -401,7 +416,7 @@ func (r *Resolver) ResolveAll(deps []*cfg.Dependency) ([]string, error) {
}

if r.ResolveAllFiles {
return r.resolveList(queue)
return r.resolveList(queue, false)
}
return r.resolveImports(queue, false)
}
Expand Down Expand Up @@ -433,6 +448,12 @@ func (r *Resolver) resolveImports(queue *list.List, testDeps bool) ([]string, er
if testDeps {
msg.Debug("Resolving test dependencies")
}

// When test deps passed in but not resolving return empty.
if testDeps && !r.ResolveTest {
return []string{}, nil
}

for e := queue.Front(); e != nil; e = e.Next() {
vdep := e.Value.(string)
dep := r.stripv(vdep)
Expand Down Expand Up @@ -505,7 +526,7 @@ func (r *Resolver) resolveImports(queue *list.List, testDeps bool) ([]string, er
// By adding to the queue it will get reprocessed now that
// it exists.
queue.PushBack(r.vpath(dep))
r.VersionHandler.SetVersion(dep)
r.VersionHandler.SetVersion(dep, testDeps)
} else if err2 != nil {
r.hadError[dep] = true
msg.Err("Error looking for %s: %s", dep, err2)
Expand Down Expand Up @@ -552,7 +573,7 @@ func (r *Resolver) resolveImports(queue *list.List, testDeps bool) ([]string, er
}
queue.PushBack(r.vpath(imp))
if err := r.Handler.InVendor(imp); err == nil {
r.VersionHandler.SetVersion(imp)
r.VersionHandler.SetVersion(imp, testDeps)
} else {
msg.Warn("Error updating %s: %s", imp, err)
}
Expand All @@ -566,7 +587,7 @@ func (r *Resolver) resolveImports(queue *list.List, testDeps bool) ([]string, er
r.alreadyQ[dep] = true
}
queue.PushBack(r.vpath(imp))
r.VersionHandler.SetVersion(imp)
r.VersionHandler.SetVersion(imp, testDeps)
} else if err != nil {
r.hadError[dep] = true
msg.Warn("Error looking for %s: %s", imp, err)
Expand All @@ -585,7 +606,7 @@ func (r *Resolver) resolveImports(queue *list.List, testDeps bool) ([]string, er
r.alreadyQ[dep] = true
}
queue.PushBack(r.vpath(imp))
r.VersionHandler.SetVersion(imp)
r.VersionHandler.SetVersion(imp, testDeps)
}
}
}
Expand Down Expand Up @@ -634,7 +655,11 @@ func (r *Resolver) resolveImports(queue *list.List, testDeps bool) ([]string, er
// This walks the entire file tree for the given dependencies, not just the
// parts that are imported directly. Using this will discover dependencies
// regardless of OS, and arch.
func (r *Resolver) resolveList(queue *list.List) ([]string, error) {
func (r *Resolver) resolveList(queue *list.List, testDeps bool) ([]string, error) {
// When test deps passed in but not resolving return empty.
if testDeps && !r.ResolveTest {
return []string{}, nil
}

var failedDep string
for e := queue.Front(); e != nil; e = e.Next() {
Expand Down Expand Up @@ -667,7 +692,7 @@ func (r *Resolver) resolveList(queue *list.List) ([]string, error) {
// Anything that comes through here has already been through
// the queue.
r.alreadyQ[path] = true
e := r.queueUnseen(path, queue)
e := r.queueUnseen(path, queue, testDeps)
if err != nil {
failedDep = path
//msg.Err("Failed to fetch dependency %s: %s", path, err)
Expand All @@ -689,6 +714,10 @@ func (r *Resolver) resolveList(queue *list.List) ([]string, error) {

// TODO(mattfarina): Need to eventually support devImport
existing := r.Config.Imports.Get(root)
if existing == nil && testDeps {
existing = r.Config.DevImports.Get(root)
}

if existing != nil {
if sp != "" && !existing.HasSubpackage(sp) {
existing.Subpackages = append(existing.Subpackages, sp)
Expand All @@ -701,7 +730,11 @@ func (r *Resolver) resolveList(queue *list.List) ([]string, error) {
newDep.Subpackages = []string{sp}
}

r.Config.Imports = append(r.Config.Imports, newDep)
if testDeps {
r.Config.DevImports = append(r.Config.DevImports, newDep)
} else {
r.Config.Imports = append(r.Config.Imports, newDep)
}
}
res = append(res, e.Value.(string))
}
Expand All @@ -711,15 +744,15 @@ func (r *Resolver) resolveList(queue *list.List) ([]string, error) {

// queueUnseenImports scans a package's imports and adds any new ones to the
// processing queue.
func (r *Resolver) queueUnseen(pkg string, queue *list.List) error {
func (r *Resolver) queueUnseen(pkg string, queue *list.List, testDeps bool) error {
// A pkg is marked "seen" as soon as we have inspected it the first time.
// Seen means that we have added all of its imports to the list.

// Already queued indicates that we've either already put it into the queue
// or intentionally not put it in the queue for fatal reasons (e.g. no
// buildable source).

deps, err := r.imports(pkg)
deps, err := r.imports(pkg, testDeps)
if err != nil && !strings.HasPrefix(err.Error(), "no buildable Go source") {
msg.Err("Could not find %s: %s", pkg, err)
return err
Expand All @@ -744,7 +777,7 @@ func (r *Resolver) queueUnseen(pkg string, queue *list.List) error {
// If the package is in GOROOT, this will return an empty list (but not
// an error).
// If it cannot resolve the pkg, it will return an error.
func (r *Resolver) imports(pkg string) ([]string, error) {
func (r *Resolver) imports(pkg string, testDeps bool) ([]string, error) {

if r.Config.HasIgnore(pkg) {
msg.Debug("Ignoring %s", pkg)
Expand Down Expand Up @@ -804,15 +837,15 @@ func (r *Resolver) imports(pkg string) ([]string, error) {
}
if found {
buf = append(buf, filepath.Join(r.VendorDir, filepath.FromSlash(imp)))
r.VersionHandler.SetVersion(imp)
r.VersionHandler.SetVersion(imp, testDeps)
continue
}
r.seen[info.Path] = true
case LocVendor:
//msg.Debug("Vendored: %s", imp)
buf = append(buf, info.Path)
if err := r.Handler.InVendor(imp); err == nil {
r.VersionHandler.SetVersion(imp)
r.VersionHandler.SetVersion(imp, testDeps)
} else {
msg.Warn("Error updating %s: %s", imp, err)
}
Expand All @@ -826,7 +859,7 @@ func (r *Resolver) imports(pkg string) ([]string, error) {
// in a less-than-perfect, but functional, situation.
if found {
buf = append(buf, filepath.Join(r.VendorDir, filepath.FromSlash(imp)))
r.VersionHandler.SetVersion(imp)
r.VersionHandler.SetVersion(imp, testDeps)
continue
}
msg.Warn("Package %s is on GOPATH, but not vendored. Ignoring.", imp)
Expand Down
5 changes: 5 additions & 0 deletions glide.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -563,6 +563,10 @@ Example:
Name: "strip-vendor, v",
Usage: "Removes nested vendor and Godeps/_workspace directories. Requires --strip-vcs.",
},
cli.BoolFlag{
Name: "skip-test",
Usage: "Resolve dependencies in test files.",
},
},
Action: func(c *cli.Context) {
if c.Bool("strip-vendor") && !c.Bool("strip-vcs") {
Expand All @@ -583,6 +587,7 @@ Example:
installer.ResolveAllFiles = c.Bool("all-dependencies")
installer.Home = c.GlobalString("home")
installer.DeleteUnused = c.Bool("delete")
installer.ResolveTest = !c.Bool("skip-test")

action.Update(installer, c.Bool("no-recursive"), c.Bool("strip-vcs"), c.Bool("strip-vendor"))
},
Expand Down
31 changes: 28 additions & 3 deletions repo/installer.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,9 @@ type Installer struct {
// packages.
ResolveAllFiles bool

// ResolveTest sets if test dependencies should be resolved.
ResolveTest bool

// Updated tracks the packages that have been remotely fetched.
Updated *UpdateTracker
}
Expand Down Expand Up @@ -524,7 +527,7 @@ func (d *VersionHandler) Process(pkg string) (e error) {
// - keeping the already set version
// - proviting messaging about the version conflict
// TODO(mattfarina): The way version setting happens can be improved. Currently not optimal.
func (d *VersionHandler) SetVersion(pkg string) (e error) {
func (d *VersionHandler) SetVersion(pkg string, testDep bool) (e error) {
root := util.GetRootFromPackage(pkg)

// Skip any references to the root package.
Expand All @@ -533,6 +536,20 @@ func (d *VersionHandler) SetVersion(pkg string) (e error) {
}

v := d.Config.Imports.Get(root)
if testDep {
if v == nil {
v = d.Config.DevImports.Get(root)
} else if d.Config.DevImports.Has(root) {
// Both imports and test imports lists the same dependency.
// There are import chains (because the import tree is resolved
// before the test tree) that can cause this.
tempD := d.Config.DevImports.Get(root)
if tempD.Reference != v.Reference {
msg.Warn("Using import %s (version %s) for test instead of testImport (version %s).", v.Name, v.Reference, tempD.Reference)
}
// TODO(mattfarina): Note repo difference in a warning.
}
}

dep, req := d.Use.Get(root)
if dep != nil && v != nil {
Expand All @@ -553,7 +570,11 @@ func (d *VersionHandler) SetVersion(pkg string) (e error) {
} else if dep != nil {
// We've got an imported dependency to use and don't already have a
// record of it. Append it to the Imports.
d.Config.Imports = append(d.Config.Imports, dep)
if testDep {
d.Config.DevImports = append(d.Config.DevImports, dep)
} else {
d.Config.Imports = append(d.Config.Imports, dep)
}
} else {
// If we've gotten here we don't have any depenency objects.
r, sp := util.NormalizeName(pkg)
Expand All @@ -563,7 +584,11 @@ func (d *VersionHandler) SetVersion(pkg string) (e error) {
if sp != "" {
dep.Subpackages = []string{sp}
}
d.Config.Imports = append(d.Config.Imports, dep)
if testDep {
d.Config.DevImports = append(d.Config.DevImports, dep)
} else {
d.Config.Imports = append(d.Config.Imports, dep)
}
}

err := VcsVersion(dep, d.Destination)
Expand Down

0 comments on commit bed8a15

Please sign in to comment.