Moving the updated tracking to the installer and making it concurrency safe
diff --git a/cfg/config.go b/cfg/config.go
index 9a9bb64..88bdc71 100644
--- a/cfg/config.go
+++ b/cfg/config.go
@@ -48,17 +48,6 @@
// 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"`
-
- // Updated tracks which packages have already been updated, so we don't
- // duplicate work.
- Updated map[string]bool `yaml:"-"`
-}
-
-// NewConfig allocates a Config structure with all fields properly initialized.
-func NewConfig() *Config {
- cfg := &Config{}
- cfg.Updated = map[string]bool{}
- return cfg
}
// A transitive representation of a dependency for importing and exporting to yaml.
@@ -75,7 +64,7 @@
// ConfigFromYaml returns an instance of Config from YAML
func ConfigFromYaml(yml []byte) (*Config, error) {
- cfg := NewConfig()
+ cfg := &Config{}
err := yaml.Unmarshal([]byte(yml), &cfg)
return cfg, err
}
@@ -167,7 +156,7 @@
// Clone performs a deep clone of the Config instance
func (c *Config) Clone() *Config {
- n := NewConfig()
+ n := &Config{}
n.Name = c.Name
n.Description = c.Description
n.Home = c.Home
@@ -176,9 +165,6 @@
n.Ignore = c.Ignore
n.Imports = c.Imports.Clone()
n.DevImports = c.DevImports.Clone()
- for k, v := range c.Updated {
- n.Updated[k] = v
- }
return n
}
diff --git a/glide.go b/glide.go
index 301c165..26dc1b5 100644
--- a/glide.go
+++ b/glide.go
@@ -219,14 +219,13 @@
msg.Warn("Only resolving dependencies for the current OS/Arch")
}
- inst := &repo.Installer{
- Force: c.Bool("force"),
- UseCache: c.Bool("cache"),
- UseGopath: c.Bool("use-gopath"),
- UseCacheGopath: c.Bool("cache-gopath"),
- UpdateVendored: c.Bool("update-vendored"),
- ResolveAllFiles: c.Bool("all-dependencies"),
- }
+ inst := repo.NewInstaller()
+ inst.Force = c.Bool("force")
+ inst.UseCache = c.Bool("cache")
+ inst.UseGopath = c.Bool("use-gopath")
+ inst.UseCacheGopath = c.Bool("cache-gopath")
+ inst.UpdateVendored = c.Bool("update-vendored")
+ inst.ResolveAllFiles = c.Bool("all-dependencies")
packages := []string(c.Args())
insecure := c.Bool("insecure")
action.Get(packages, inst, insecure, c.Bool("no-recursive"))
@@ -259,10 +258,8 @@
// FIXME: Implement this in the installer.
fmt.Println("Delete is not currently implemented.")
}
-
- inst := &repo.Installer{
- Force: c.Bool("force"),
- }
+ inst := repo.NewInstaller()
+ inst.Force = c.Bool("force")
packages := []string(c.Args())
action.Remove(packages, inst)
},
@@ -396,15 +393,14 @@
},
},
Action: func(c *cli.Context) {
- installer := &repo.Installer{
- DeleteUnused: c.Bool("deleteOptIn"),
- UpdateVendored: c.Bool("update-vendored"),
- Force: c.Bool("force"),
- UseCache: c.Bool("cache"),
- UseCacheGopath: c.Bool("cache-gopath"),
- UseGopath: c.Bool("use-gopath"),
- Home: gpath.Home(),
- }
+ installer := repo.NewInstaller()
+ installer.Force = c.Bool("force")
+ installer.UseCache = c.Bool("cache")
+ installer.UseGopath = c.Bool("use-gopath")
+ installer.UseCacheGopath = c.Bool("cache-gopath")
+ installer.UpdateVendored = c.Bool("update-vendored")
+ installer.Home = gpath.Home()
+ installer.DeleteUnused = c.Bool("deleteOptIn")
action.Install(installer)
},
@@ -487,16 +483,15 @@
msg.Warn("Only resolving dependencies for the current OS/Arch")
}
- installer := &repo.Installer{
- DeleteUnused: c.Bool("deleteOptIn"),
- UpdateVendored: c.Bool("update-vendored"),
- ResolveAllFiles: c.Bool("all-dependencies"),
- Force: c.Bool("force"),
- UseCache: c.Bool("cache"),
- UseCacheGopath: c.Bool("cache-gopath"),
- UseGopath: c.Bool("use-gopath"),
- Home: gpath.Home(),
- }
+ installer := repo.NewInstaller()
+ installer.Force = c.Bool("force")
+ installer.UseCache = c.Bool("cache")
+ installer.UseGopath = c.Bool("use-gopath")
+ installer.UseCacheGopath = c.Bool("cache-gopath")
+ installer.UpdateVendored = c.Bool("update-vendored")
+ installer.ResolveAllFiles = c.Bool("all-dependencies")
+ installer.Home = gpath.Home()
+ installer.DeleteUnused = c.Bool("deleteOptIn")
action.Update(installer, c.Bool("no-recursive"))
},
diff --git a/repo/installer.go b/repo/installer.go
index 74b7532..bebc3df 100644
--- a/repo/installer.go
+++ b/repo/installer.go
@@ -47,6 +47,15 @@
// of every file of every package, rather than only following imported
// packages.
ResolveAllFiles bool
+
+ // Updated tracks the packages that have been remotely fetched.
+ Updated *UpdateTracker
+}
+
+func NewInstaller() *Installer {
+ i := &Installer{}
+ i.Updated = NewUpdateTracker()
+ return i
}
// VendorPath returns the path to the location to put vendor packages
@@ -109,8 +118,8 @@
return newConf, nil
}
- ConcurrentUpdate(newConf.Imports, cwd, i, conf.Updated)
- ConcurrentUpdate(newConf.DevImports, cwd, i, conf.Updated)
+ ConcurrentUpdate(newConf.Imports, cwd, i)
+ ConcurrentUpdate(newConf.DevImports, cwd, i)
return newConf, nil
}
@@ -122,12 +131,12 @@
dest := i.VendorPath()
- if err := ConcurrentUpdate(conf.Imports, dest, i, conf.Updated); err != nil {
+ if err := ConcurrentUpdate(conf.Imports, dest, i); err != nil {
return err
}
if useDev {
- return ConcurrentUpdate(conf.DevImports, dest, i, conf.Updated)
+ return ConcurrentUpdate(conf.DevImports, dest, i)
}
return nil
@@ -157,6 +166,7 @@
updateVendored: i.UpdateVendored,
Config: conf,
Use: ic,
+ updated: i.Updated,
}
v := &VersionHandler{
@@ -186,7 +196,7 @@
msg.Warn("dev imports not resolved.")
}
- err = ConcurrentUpdate(conf.Imports, vpath, i, conf.Updated)
+ err = ConcurrentUpdate(conf.Imports, vpath, i)
return err
}
@@ -229,7 +239,7 @@
}
// ConcurrentUpdate takes a list of dependencies and updates in parallel.
-func ConcurrentUpdate(deps []*cfg.Dependency, cwd string, i *Installer, updated map[string]bool) error {
+func ConcurrentUpdate(deps []*cfg.Dependency, cwd string, i *Installer) error {
done := make(chan struct{}, concurrentWorkers)
in := make(chan *cfg.Dependency, concurrentWorkers)
var wg sync.WaitGroup
@@ -244,7 +254,7 @@
select {
case dep := <-ch:
dest := filepath.Join(i.VendorPath(), dep.Name)
- if err := VcsUpdate(dep, dest, i.Home, i.UseCache, i.UseCacheGopath, i.UseGopath, i.Force, i.UpdateVendored, updated); err != nil {
+ if err := VcsUpdate(dep, dest, i.Home, i.UseCache, i.UseCacheGopath, i.UseGopath, i.Force, i.UpdateVendored, i.Updated); err != nil {
msg.Err("Update failed for %s: %s\n", dep.Name, err)
// Capture the error while making sure the concurrent
// operations don't step on each other.
@@ -312,6 +322,7 @@
cache, cacheGopath, useGopath, force, updateVendored bool
Config *cfg.Config
Use *importCache
+ updated *UpdateTracker
}
// NotFound attempts to retrieve a package when not found in the local vendor/
@@ -429,7 +440,7 @@
m.Config.Imports = append(m.Config.Imports, d)
}
- if err := VcsUpdate(d, dest, m.home, m.cache, m.cacheGopath, m.useGopath, m.force, m.updateVendored, m.Config.Updated); err != nil {
+ if err := VcsUpdate(d, dest, m.home, m.cache, m.cacheGopath, m.useGopath, m.force, m.updateVendored, m.updated); err != nil {
return err
}
diff --git a/repo/tracker.go b/repo/tracker.go
new file mode 100644
index 0000000..c017ffd
--- /dev/null
+++ b/repo/tracker.go
@@ -0,0 +1,42 @@
+package repo
+
+import (
+ "sync"
+)
+
+// UpdateTracker holds a list of all the packages that have been updated from
+// an external source. This is a concurrency safe implementation.
+type UpdateTracker struct {
+ sync.RWMutex
+
+ updated map[string]bool
+}
+
+// NewUpdateTracker creates a new instance of UpdateTracker ready for use.
+func NewUpdateTracker() *UpdateTracker {
+ u := &UpdateTracker{}
+ u.updated = map[string]bool{}
+ return u
+}
+
+// Add adds a name to the list of items being tracked.
+func (u *UpdateTracker) Add(name string) {
+ u.Lock()
+ u.updated[name] = true
+ u.Unlock()
+}
+
+// Check returns if an item is on the list or not.
+func (u *UpdateTracker) Check(name string) bool {
+ u.RLock()
+ _, f := u.updated[name]
+ u.RUnlock()
+ return f
+}
+
+// Remove takes a package off the list
+func (u *UpdateTracker) Remove(name string) {
+ u.Lock()
+ delete(u.updated, name)
+ u.Unlock()
+}
diff --git a/repo/tracker_test.go b/repo/tracker_test.go
new file mode 100644
index 0000000..94150f2
--- /dev/null
+++ b/repo/tracker_test.go
@@ -0,0 +1,23 @@
+package repo
+
+import "testing"
+
+func TestUpdateTracker(t *testing.T) {
+ tr := NewUpdateTracker()
+
+ if f := tr.Check("github.com/foo/bar"); f != false {
+ t.Error("Error, package Check passed on empty tracker")
+ }
+
+ tr.Add("github.com/foo/bar")
+
+ if f := tr.Check("github.com/foo/bar"); f != true {
+ t.Error("Error, failed to add package to tracker")
+ }
+
+ tr.Remove("github.com/foo/bar")
+
+ if f := tr.Check("github.com/foo/bar"); f != false {
+ t.Error("Error, failed to remove package from tracker")
+ }
+}
diff --git a/repo/vcs.go b/repo/vcs.go
index 992b883..03267d8 100644
--- a/repo/vcs.go
+++ b/repo/vcs.go
@@ -21,7 +21,7 @@
)
// VcsUpdate updates to a particular checkout based on the VCS setting.
-func VcsUpdate(dep *cfg.Dependency, dest, home string, cache, cacheGopath, useGopath, force, updateVendored bool, updated map[string]bool) error {
+func VcsUpdate(dep *cfg.Dependency, dest, home string, cache, cacheGopath, useGopath, force, updateVendored bool, updated *UpdateTracker) error {
// If the dependency has already been pinned we can skip it. This is a
// faster path so we don't need to resolve it again.
@@ -30,11 +30,11 @@
return nil
}
- if updated[dep.Name] {
- msg.Debug("%s was already updated, skipping.\n", dep.Name)
+ if updated.Check(dep.Name) {
+ msg.Debug("%s was already updated, skipping.", dep.Name)
return nil
}
- updated[dep.Name] = true
+ updated.Add(dep.Name)
msg.Info("Fetching updates for %s.\n", dep.Name)