Fixed #517 failed to install testImport from lock
There were two reasons for this
1. The case there import was empty but testImport was not
2. When the same dependency appeared in both import and testImport
thre was a race condition that could cause failure
diff --git a/CHANGELOG.md b/CHANGELOG.md
index fac8d08..68828a2 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -9,6 +9,8 @@
This skips unnecessary network requests (thanks @hori-ryota)
## Fixed
+- #517: Fixed failure to install testImport from lock when no imports present
+ or when same dependency on both import and testImport
- #440: Fixed panic in `glide tree` when walking the filesystem (thanks @abhin4v)
- #529: --delete flag deleted and re-downloaded transitive dependencies
- #535: Resolve vendor directory symlinks (thanks @Fugiman)
diff --git a/action/get.go b/action/get.go
index 5c34d5e..009d90d 100644
--- a/action/get.go
+++ b/action/get.go
@@ -115,7 +115,10 @@
if err != nil {
msg.Die("Failed to generate config hash. Unable to generate lock file.")
}
- lock := cfg.NewLockfile(confcopy.Imports, confcopy.DevImports, hash)
+ lock, err := cfg.NewLockfile(confcopy.Imports, confcopy.DevImports, hash)
+ if err != nil {
+ msg.Die("Failed to generate lock file: %s", err)
+ }
if err := lock.WriteFile(filepath.Join(base, gpath.LockFile)); err != nil {
msg.Die("Failed to write glide lock file: %s", err)
}
diff --git a/action/update.go b/action/update.go
index bdd3c62..9712ebb 100644
--- a/action/update.go
+++ b/action/update.go
@@ -87,7 +87,10 @@
if err != nil {
msg.Die("Failed to generate config hash. Unable to generate lock file.")
}
- lock := cfg.NewLockfile(confcopy.Imports, confcopy.DevImports, hash)
+ lock, err := cfg.NewLockfile(confcopy.Imports, confcopy.DevImports, hash)
+ if err != nil {
+ msg.Die("Failed to generate lock file: %s", err)
+ }
wl := true
if gpath.HasLock(base) {
yml, err := ioutil.ReadFile(filepath.Join(base, gpath.LockFile))
diff --git a/cfg/lock.go b/cfg/lock.go
index 8a26e24..62d08ef 100644
--- a/cfg/lock.go
+++ b/cfg/lock.go
@@ -2,6 +2,7 @@
import (
"crypto/sha256"
+ "fmt"
"io/ioutil"
"sort"
"strings"
@@ -40,6 +41,27 @@
for _, imp := range lf.Imports {
sort.Strings(imp.Subpackages)
}
+
+ // Ensure elements on testImport don't already exist on import.
+ var newDI Locks
+ var found bool
+ for _, imp := range lf.DevImports {
+ found = false
+ for i := 0; i < len(lf.Imports); i++ {
+ if lf.Imports[i].Name == imp.Name {
+ found = true
+ if lf.Imports[i].Version != imp.Version {
+ return lf, fmt.Errorf("Generating lock YAML produced conflicting versions of %s. import (%s), testImport (%s)", imp.Name, lf.Imports[i].Version, imp.Version)
+ }
+ }
+ }
+
+ if !found {
+ newDI = append(newDI, imp)
+ }
+ }
+ lf.DevImports = newDI
+
for _, imp := range lf.DevImports {
sort.Strings(imp.Subpackages)
}
@@ -169,7 +191,7 @@
}
// NewLockfile is used to create an instance of Lockfile.
-func NewLockfile(ds, tds Dependencies, hash string) *Lockfile {
+func NewLockfile(ds, tds Dependencies, hash string) (*Lockfile, error) {
lf := &Lockfile{
Hash: hash,
Updated: time.Now(),
@@ -183,13 +205,26 @@
sort.Sort(lf.Imports)
+ var found bool
for i := 0; i < len(tds); i++ {
- lf.DevImports[i] = LockFromDependency(tds[i])
+ found = false
+ for ii := 0; ii < len(ds); ii++ {
+ if ds[ii].Name == tds[i].Name {
+ found = true
+ if ds[ii].Reference != tds[i].Reference {
+ return &Lockfile{}, fmt.Errorf("Generating lock produced conflicting versions of %s. import (%s), testImport (%s)", tds[i].Name, ds[ii].Reference, tds[i].Reference)
+ }
+ break
+ }
+ }
+ if !found {
+ lf.DevImports[i] = LockFromDependency(tds[i])
+ }
}
sort.Sort(lf.DevImports)
- return lf
+ return lf, nil
}
// LockfileFromMap takes a map of dependencies and generates a lock Lockfile instance.
diff --git a/repo/installer.go b/repo/installer.go
index 13d3fb0..5fd8562 100644
--- a/repo/installer.go
+++ b/repo/installer.go
@@ -104,8 +104,8 @@
newConf.DeDupe()
- if len(newConf.Imports) == 0 {
- msg.Info("No dependencies found. Nothing installed.\n")
+ if len(newConf.Imports) == 0 && len(newConf.DevImports) == 0 {
+ msg.Info("No dependencies found. Nothing installed.")
return newConf, nil
}
diff --git a/repo/set_reference.go b/repo/set_reference.go
index 4363459..f5891ac 100644
--- a/repo/set_reference.go
+++ b/repo/set_reference.go
@@ -3,9 +3,11 @@
import (
"sync"
+ "github.com/Masterminds/glide/cache"
"github.com/Masterminds/glide/cfg"
"github.com/Masterminds/glide/msg"
gpath "github.com/Masterminds/glide/path"
+ "github.com/codegangsta/cli"
)
// SetReference is a command to set the VCS reference (commit id, tag, etc) for
@@ -25,15 +27,39 @@
done := make(chan struct{}, concurrentWorkers)
in := make(chan *cfg.Dependency, concurrentWorkers)
var wg sync.WaitGroup
+ var lock sync.Mutex
+ var returnErr error
for i := 0; i < concurrentWorkers; i++ {
go func(ch <-chan *cfg.Dependency) {
for {
select {
case dep := <-ch:
+ var loc string
+ if dep.Repository != "" {
+ loc = dep.Repository
+ } else {
+ loc = "https://" + dep.Name
+ }
+ key, err := cache.Key(loc)
+ if err != nil {
+ msg.Die(err.Error())
+ }
+ cache.Lock(key)
if err := VcsVersion(dep, cwd); err != nil {
msg.Err("Failed to set version on %s to %s: %s\n", dep.Name, dep.Reference, err)
+
+ // Capture the error while making sure the concurrent
+ // operations don't step on each other.
+ lock.Lock()
+ if returnErr == nil {
+ returnErr = err
+ } else {
+ returnErr = cli.NewMultiError(returnErr, err)
+ }
+ lock.Unlock()
}
+ cache.Unlock(key)
wg.Done()
case <-done:
return
@@ -66,5 +92,5 @@
// close(done)
// close(in)
- return nil
+ return returnErr
}