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)