Chase gps again to add prefetching
diff --git a/glide.lock b/glide.lock
index e9e7e6b..9a968e2 100644
--- a/glide.lock
+++ b/glide.lock
@@ -10,7 +10,7 @@
- name: github.com/Masterminds/vcs
version: fbe9fb6ad5b5f35b3e82a7c21123cfc526cbf895
- name: github.com/sdboyer/gps
- version: 63a033c13497e9f5ca47f5f6d4e02455f5d4d85d
+ version: 278a227dfc3d595a33a77ff3f841fd8ca1bc8cd0
- name: github.com/termie/go-shutil
version: bcacb06fecaeec8dc42af03c87c6949f4a05c74c
- name: gopkg.in/yaml.v2
diff --git a/vendor/github.com/sdboyer/gps/analysis.go b/vendor/github.com/sdboyer/gps/analysis.go
index 0cb93ba..7fcb5bf 100644
--- a/vendor/github.com/sdboyer/gps/analysis.go
+++ b/vendor/github.com/sdboyer/gps/analysis.go
@@ -426,10 +426,12 @@
return true
case grey:
- // grey means an import cycle; guaranteed badness right here.
+ // grey means an import cycle; guaranteed badness right here. You'd
+ // hope we never encounter it in a dependency (really? you published
+ // that code?), but we have to defend against it.
//
- // FIXME handle import cycles by dropping everything involved. i
- // think we need to compute SCC, then drop *all* of them?
+ // FIXME handle import cycles by dropping everything involved. (i
+ // think we need to compute SCC, then drop *all* of them?)
colors[pkg] = black
poison(append(path, pkg)) // poison self and parents
@@ -730,12 +732,14 @@
}
// ExternalReach looks through a PackageTree and computes the list of external
-// packages (not logical children of PackageTree.ImportRoot) that are
-// transitively imported by the internal packages in the tree.
+// import statements (that is, import statements pointing to packages that are
+// not logical children of PackageTree.ImportRoot) that are transitively
+// imported by the internal packages in the tree.
//
// main indicates whether (true) or not (false) to include main packages in the
-// analysis. main packages are generally excluded when analyzing anything other
-// than the root project, as they inherently can't be imported.
+// analysis. When utilized by gps' solver, main packages are generally excluded
+// from analyzing anything other than the root project, as they necessarily can't
+// be imported.
//
// tests indicates whether (true) or not (false) to include imports from test
// files in packages when computing the reach map.
@@ -744,25 +748,35 @@
// analysis. This exclusion applies to both internal and external packages. If
// an external import path is ignored, it is simply omitted from the results.
//
-// If an internal path is ignored, then it is excluded from all transitive
-// dependency chains and does not appear as a key in the final map. That is, if
-// you ignore A/foo, then the external package list for all internal packages
-// that import A/foo will not include external packages that are only reachable
-// through A/foo.
+// If an internal path is ignored, then not only does it not appear in the final
+// map, but it is also excluded from the transitive calculations of other
+// internal packages. That is, if you ignore A/foo, then the external package
+// list for all internal packages that import A/foo will not include external
+// packages that are only reachable through A/foo.
//
// Visually, this means that, given a PackageTree with root A and packages at A,
// A/foo, and A/bar, and the following import chain:
//
// A -> A/foo -> A/bar -> B/baz
//
-// If you ignore A/foo, then the returned map would be:
+// In this configuration, all of A's packages transitively import B/baz, so the
+// returned map would be:
+//
+// map[string][]string{
+// "A": []string{"B/baz"},
+// "A/foo": []string{"B/baz"}
+// "A/bar": []string{"B/baz"},
+// }
+//
+// However, if you ignore A/foo, then A's path to B/baz is broken, and A/foo is
+// omitted entirely. Thus, the returned map would be:
//
// map[string][]string{
// "A": []string{},
// "A/bar": []string{"B/baz"},
// }
//
-// It is safe to pass a nil map if there are no packages to ignore.
+// If there are no packages to ignore, it is safe to pass a nil map.
func (t PackageTree) ExternalReach(main, tests bool, ignore map[string]bool) map[string][]string {
if ignore == nil {
ignore = make(map[string]bool)
diff --git a/vendor/github.com/sdboyer/gps/bridge.go b/vendor/github.com/sdboyer/gps/bridge.go
index 2aae74b..298b023 100644
--- a/vendor/github.com/sdboyer/gps/bridge.go
+++ b/vendor/github.com/sdboyer/gps/bridge.go
@@ -5,6 +5,7 @@
"os"
"path/filepath"
"sort"
+ "sync/atomic"
"github.com/Masterminds/semver"
)
@@ -21,6 +22,7 @@
matches(id ProjectIdentifier, c Constraint, v Version) bool
matchesAny(id ProjectIdentifier, c1, c2 Constraint) bool
intersect(id ProjectIdentifier, c1, c2 Constraint) Constraint
+ breakLock()
}
// bridge is an adapter around a proper SourceManager. It provides localized
@@ -51,6 +53,9 @@
// is that this keeps the versions sorted in the direction required by the
// current solve run
vlists map[ProjectIdentifier][]Version
+
+ // Indicates whether lock breaking has already been run
+ lockbroken int32
}
// Global factory func to create a bridge. This exists solely to allow tests to
@@ -413,6 +418,43 @@
return b.sm.DeduceProjectRoot(ip)
}
+// breakLock is called when the solver has to break a version recorded in the
+// lock file. It prefetches all the projects in the solver's lock , so that the
+// information is already on hand if/when the solver needs it.
+//
+// Projects that have already been selected are skipped, as it's generally unlikely that the
+// solver will have to backtrack through and fully populate their version queues.
+func (b *bridge) breakLock() {
+ // No real conceivable circumstance in which multiple calls are made to
+ // this, but being that this is the entrance point to a bunch of async work,
+ // protect it with an atomic CAS in case things change in the future.
+ if !atomic.CompareAndSwapInt32(&b.lockbroken, 0, 1) {
+ return
+ }
+
+ for _, lp := range b.s.rl.Projects() {
+ if _, is := b.s.sel.selected(lp.pi); !is {
+ // ListPackages guarantees that all the necessary network work will
+ // be done, so go with that
+ //
+ // TODO(sdboyer) use this as an opportunity to detect
+ // inconsistencies between upstream and the lock (e.g., moved tags)?
+ pi, v := lp.pi, lp.Version()
+ go func() {
+ // Sync first
+ b.sm.SyncSourceFor(pi)
+ // Preload the package info for the locked version, too, as
+ // we're more likely to need that
+ b.sm.ListPackages(pi, v)
+ }()
+ }
+ }
+}
+
+func (b *bridge) SyncSourceFor(id ProjectIdentifier) error {
+ return b.sm.SyncSourceFor(id)
+}
+
// versionTypeUnion represents a set of versions that are, within the scope of
// this solver run, equivalent.
//
diff --git a/vendor/github.com/sdboyer/gps/codecov.yml b/vendor/github.com/sdboyer/gps/codecov.yml
index 263381f..cdc5202 100644
--- a/vendor/github.com/sdboyer/gps/codecov.yml
+++ b/vendor/github.com/sdboyer/gps/codecov.yml
@@ -2,4 +2,5 @@
ignore:
- remove_go16.go
- remove_go17.go
- - errors.go
+ - solve_failures.go
+ - discovery.go # copied from stdlib, don't need to test
diff --git a/vendor/github.com/sdboyer/gps/manager_test.go b/vendor/github.com/sdboyer/gps/manager_test.go
index 439d8b4..df85293 100644
--- a/vendor/github.com/sdboyer/gps/manager_test.go
+++ b/vendor/github.com/sdboyer/gps/manager_test.go
@@ -98,7 +98,7 @@
}
}
-func TestProjectManagerInit(t *testing.T) {
+func TestSourceInit(t *testing.T) {
// This test is a bit slow, skip it on -short
if testing.Short() {
t.Skip("Skipping project manager init test in short mode")
@@ -165,10 +165,10 @@
t.Errorf("Unexpected error during initial project setup/fetching %s", err)
}
+ rev := Revision("30605f6ac35fcb075ad0bfa9296f90a7d891523e")
if len(v) != 3 {
t.Errorf("Expected three version results from the test repo, got %v", len(v))
} else {
- rev := Revision("30605f6ac35fcb075ad0bfa9296f90a7d891523e")
expected := []Version{
NewVersion("1.0.0").Is(rev),
NewBranch("master").Is(rev),
@@ -182,9 +182,18 @@
}
}
- // use ListPackages to ensure the repo is actually on disk
- // TODO(sdboyer) ugh, maybe we do need an explicit prefetch method
- smc.ListPackages(id, NewVersion("1.0.0"))
+ present, err := smc.RevisionPresentIn(id, rev)
+ if err != nil {
+ t.Errorf("Should have found revision in source, but got err: %s", err)
+ } else if !present {
+ t.Errorf("Should have found revision in source, but did not")
+ }
+
+ // SyncSourceFor will ensure we have everything
+ err = smc.SyncSourceFor(id)
+ if err != nil {
+ t.Errorf("SyncSourceFor failed with unexpected error: %s", err)
+ }
// Ensure that the appropriate cache dirs and files exist
_, err = os.Stat(filepath.Join(cpath, "sources", "https---github.com-Masterminds-VCSTestRepo", ".git"))
@@ -209,6 +218,36 @@
}
}
+func TestMgrMethodsFailWithBadPath(t *testing.T) {
+ // a symbol will always bork it up
+ bad := mkPI("foo/##&^")
+ sm, clean := mkNaiveSM(t)
+ defer clean()
+
+ var err error
+ if _, err = sm.SourceExists(bad); err == nil {
+ t.Error("SourceExists() did not error on bad input")
+ }
+ if err = sm.SyncSourceFor(bad); err == nil {
+ t.Error("SyncSourceFor() did not error on bad input")
+ }
+ if _, err = sm.ListVersions(bad); err == nil {
+ t.Error("ListVersions() did not error on bad input")
+ }
+ if _, err = sm.RevisionPresentIn(bad, Revision("")); err == nil {
+ t.Error("RevisionPresentIn() did not error on bad input")
+ }
+ if _, err = sm.ListPackages(bad, nil); err == nil {
+ t.Error("ListPackages() did not error on bad input")
+ }
+ if _, _, err = sm.GetManifestAndLock(bad, nil); err == nil {
+ t.Error("GetManifestAndLock() did not error on bad input")
+ }
+ if err = sm.ExportProject(bad, nil, ""); err == nil {
+ t.Error("ExportProject() did not error on bad input")
+ }
+}
+
func TestGetSources(t *testing.T) {
// This test is a tad slow, skip it on -short
if testing.Short() {
diff --git a/vendor/github.com/sdboyer/gps/solve_basic_test.go b/vendor/github.com/sdboyer/gps/solve_basic_test.go
index 6b6a092..e4c1352 100644
--- a/vendor/github.com/sdboyer/gps/solve_basic_test.go
+++ b/vendor/github.com/sdboyer/gps/solve_basic_test.go
@@ -1293,6 +1293,14 @@
return false, nil
}
+func (sm *depspecSourceManager) SyncSourceFor(id ProjectIdentifier) error {
+ // Ignore err because it can't happen
+ if exist, _ := sm.SourceExists(id); !exist {
+ return fmt.Errorf("Source %s does not exist", id.errString())
+ }
+ return nil
+}
+
func (sm *depspecSourceManager) VendorCodeExists(id ProjectIdentifier) (bool, error) {
return false, nil
}
diff --git a/vendor/github.com/sdboyer/gps/solver.go b/vendor/github.com/sdboyer/gps/solver.go
index d82a40c..507133b 100644
--- a/vendor/github.com/sdboyer/gps/solver.go
+++ b/vendor/github.com/sdboyer/gps/solver.go
@@ -169,8 +169,8 @@
// else fail with an informative error.
//
// If a Solution is found, an implementing tool may persist it - typically into
-// what a "lock file" - and/or use it to write out a directory tree of
-// dependencies, suitable to be a vendor directory, via CreateVendorTree.
+// a "lock file" - and/or use it to write out a directory tree of dependencies,
+// suitable to be a vendor directory, via CreateVendorTree.
type Solver interface {
// HashInputs produces a hash digest representing the unique inputs to this
// solver. It is guaranteed that, if the hash digest is equal to the digest
@@ -179,6 +179,9 @@
//
// In such a case, it may not be necessary to run Solve() at all.
HashInputs() ([]byte, error)
+
+ // Solve initiates a solving run. It will either complete successfully with
+ // a Solution, or fail with an informative error.
Solve() (Solution, error)
}
@@ -474,6 +477,13 @@
}
for _, dep := range deps {
+ // If we have no lock, or if this dep isn't in the lock, then prefetch
+ // it. See explanation longer comment in selectRoot() for how we benefit
+ // from parallelism here.
+ if _, has := s.rlm[dep.Ident]; !has {
+ go s.b.SyncSourceFor(dep.Ident)
+ }
+
s.sel.pushDep(dependency{depender: pa, dep: dep})
// Add all to unselected queue
s.names[dep.Ident.ProjectRoot] = dep.Ident.netName()
@@ -817,6 +827,8 @@
// though, then we have to try to use what's in the lock, because that's
// the only version we'll be able to get.
if exist, _ := s.b.SourceExists(id); exist {
+ // Upgrades mean breaking the lock
+ s.b.breakLock()
return nil, nil
}
@@ -864,6 +876,8 @@
}
if !found {
+ // No match found, which means we're going to be breaking the lock
+ s.b.breakLock()
return nil, nil
}
}
@@ -1060,6 +1074,10 @@
// If this atom has a lock, pull it out so that we can potentially inject
// preferred versions into any bmis we enqueue
+ //
+ // TODO(sdboyer) making this call here could be the first thing to trigger
+ // network activity...maybe? if so, can we mitigate by deferring the work to
+ // queue consumption time?
_, l, _ := s.b.GetManifestAndLock(a.a.id, a.a.v)
var lmap map[ProjectIdentifier]Version
if l != nil {
@@ -1070,13 +1088,31 @@
}
for _, dep := range deps {
+ // If this is dep isn't in the lock, do some prefetching. (If it is, we
+ // might be able to get away with zero network activity for it, so don't
+ // prefetch). This provides an opportunity for some parallelism wins, on
+ // two fronts:
+ //
+ // 1. Because this loop may have multiple deps in it, we could end up
+ // simultaneously fetching both in the background while solving proceeds
+ //
+ // 2. Even if only one dep gets prefetched here, the worst case is that
+ // that same dep comes out of the unselected queue next, and we gain a
+ // few microseconds before blocking later. Best case, the dep doesn't
+ // come up next, but some other dep comes up that wasn't prefetched, and
+ // both fetches proceed in parallel.
+ if _, has := s.rlm[dep.Ident]; !has {
+ go s.b.SyncSourceFor(dep.Ident)
+ }
+
s.sel.pushDep(dependency{depender: a.a, dep: dep})
// Go through all the packages introduced on this dep, selecting only
- // the ones where the only depper on them is what we pushed in. Then,
- // put those into the unselected queue.
+ // the ones where the only depper on them is what the previous line just
+ // pushed in. Then, put those into the unselected queue.
rpm := s.sel.getRequiredPackagesIn(dep.Ident)
var newp []string
for _, pkg := range dep.pl {
+ // Just one means that the dep we're visiting is the sole importer.
if rpm[pkg] == 1 {
newp = append(newp, pkg)
}
diff --git a/vendor/github.com/sdboyer/gps/source.go b/vendor/github.com/sdboyer/gps/source.go
index feaba15..6256c51 100644
--- a/vendor/github.com/sdboyer/gps/source.go
+++ b/vendor/github.com/sdboyer/gps/source.go
@@ -1,8 +1,12 @@
package gps
-import "fmt"
+import (
+ "fmt"
+ "sync"
+)
type source interface {
+ syncLocal() error
checkExistence(sourceExistence) bool
exportVersionTo(Version, string) error
getManifestAndLock(ProjectRoot, Version) (Manifest, Lock, error)
@@ -54,9 +58,6 @@
// ProjectAnalyzer used to fulfill getManifestAndLock
an ProjectAnalyzer
- // Whether the cache has the latest info on versions
- cvsync bool
-
// The project metadata cache. This is (or is intended to be) persisted to
// disk, for reuse across solver runs.
dc *sourceMetaCache
@@ -64,6 +65,20 @@
// lvfunc allows the other vcs source types that embed this type to inject
// their listVersions func into the baseSource, for use as needed.
lvfunc func() (vlist []Version, err error)
+
+ // lock to serialize access to syncLocal
+ synclock sync.Mutex
+
+ // Globalish flag indicating whether a "full" sync has been performed. Also
+ // used as a one-way gate to ensure that the full syncing routine is never
+ // run more than once on a given source instance.
+ allsync bool
+
+ // The error, if any, that occurred on syncLocal
+ syncerr error
+
+ // Whether the cache has the latest info on versions
+ cvsync bool
}
func (bs *baseVCSSource) getManifestAndLock(r ProjectRoot, v Version) (Manifest, Lock, error) {
@@ -201,7 +216,7 @@
bs.crepo.mut.Unlock()
if err != nil {
- return fmt.Errorf("failed to create repository cache for %s", bs.crepo.r.Remote())
+ return fmt.Errorf("failed to create repository cache for %s with err:\n%s", bs.crepo.r.Remote(), err)
}
bs.crepo.synced = true
bs.ex.s |= existsInCache
@@ -247,6 +262,44 @@
return ex&bs.ex.f == ex
}
+// syncLocal ensures the local data we have about the source is fully up to date
+// with what's out there over the network.
+func (bs *baseVCSSource) syncLocal() error {
+ // Ensure we only have one goroutine doing this at a time
+ bs.synclock.Lock()
+ defer bs.synclock.Unlock()
+
+ // ...and that we only ever do it once
+ if bs.allsync {
+ // Return the stored err, if any
+ return bs.syncerr
+ }
+
+ bs.allsync = true
+ // First, ensure the local instance exists
+ bs.syncerr = bs.ensureCacheExistence()
+ if bs.syncerr != nil {
+ return bs.syncerr
+ }
+
+ _, bs.syncerr = bs.lvfunc()
+ if bs.syncerr != nil {
+ return bs.syncerr
+ }
+
+ // This case is really just for git repos, where the lvfunc doesn't
+ // guarantee that the local repo is synced
+ if !bs.crepo.synced {
+ bs.syncerr = bs.crepo.r.Update()
+ if bs.syncerr != nil {
+ return bs.syncerr
+ }
+ bs.crepo.synced = true
+ }
+
+ return nil
+}
+
func (bs *baseVCSSource) listPackages(pr ProjectRoot, v Version) (ptree PackageTree, err error) {
if err = bs.ensureCacheExistence(); err != nil {
return
diff --git a/vendor/github.com/sdboyer/gps/source_manager.go b/vendor/github.com/sdboyer/gps/source_manager.go
index 11ec567..dc5a7ef 100644
--- a/vendor/github.com/sdboyer/gps/source_manager.go
+++ b/vendor/github.com/sdboyer/gps/source_manager.go
@@ -28,6 +28,10 @@
// SourceManager's central repository cache.
SourceExists(ProjectIdentifier) (bool, error)
+ // SyncSourceFor will attempt to bring all local information about a source
+ // fully up to date.
+ SyncSourceFor(ProjectIdentifier) error
+
// ListVersions retrieves a list of the available versions for a given
// repository name.
ListVersions(ProjectIdentifier) ([]Version, error)
@@ -217,6 +221,19 @@
return src.checkExistence(existsInCache) || src.checkExistence(existsUpstream), nil
}
+// SyncSourceFor will ensure that all local caches and information about a
+// source are up to date with any network-acccesible information.
+//
+// The primary use case for this is prefetching.
+func (sm *SourceMgr) SyncSourceFor(id ProjectIdentifier) error {
+ src, err := sm.getSourceFor(id)
+ if err != nil {
+ return err
+ }
+
+ return src.syncLocal()
+}
+
// ExportProject writes out the tree of the provided ProjectIdentifier's
// ProjectRoot, at the provided version, to the provided directory.
func (sm *SourceMgr) ExportProject(id ProjectIdentifier, v Version, to string) error {
diff --git a/vendor/github.com/sdboyer/gps/types.go b/vendor/github.com/sdboyer/gps/types.go
index b40807d..657d786 100644
--- a/vendor/github.com/sdboyer/gps/types.go
+++ b/vendor/github.com/sdboyer/gps/types.go
@@ -15,8 +15,8 @@
// a project's manifest, and apply to all packages in a ProjectRoot's tree.
// Solving itself mostly proceeds on a project-by-project basis.
//
-// Aliasing string types is usually a bit of an anti-pattern. We do it here as a
-// means of clarifying API intent. This is important because Go's package
+// Aliasing string types is usually a bit of an anti-pattern. gps does it here
+// as a means of clarifying API intent. This is important because Go's package
// management domain has lots of different path-ish strings floating around:
//
// actual directories:
@@ -41,9 +41,9 @@
// to, but differs in two keys ways from, an import path.
//
// First, ProjectIdentifiers do not identify a single package. Rather, they
-// encompasses the whole tree of packages that exist at or below their
+// encompasses the whole tree of packages rooted at and including their
// ProjectRoot. In gps' current design, this ProjectRoot must correspond to the
-// root of a repository, though this may not always be the case.
+// root of a repository, though this may change in the future.
//
// Second, ProjectIdentifiers can optionally carry a NetworkName, which
// identifies where the underlying source code can be located on the network.
@@ -57,7 +57,8 @@
//
// With plain import paths, network addresses are derived purely through an
// algorithm. By having an explicit network name, it becomes possible to, for
-// example, transparently substitute a fork for an original upstream repository.
+// example, transparently substitute a fork for the original upstream source
+// repository.
//
// Note that gps makes no guarantees about the actual import paths contained in
// a repository aligning with ImportRoot. If tools, or their users, specify an
@@ -126,7 +127,8 @@
return i
}
-// ProjectProperties comprise the properties that can attached to a ProjectRoot.
+// ProjectProperties comprise the properties that can be attached to a
+// ProjectRoot.
//
// In general, these are declared in the context of a map of ProjectRoot to its
// ProjectProperties; they make little sense without their corresponding
