Merge pull request #714 from jmank88/project_ident_sort

Consolidate ProjectIndentifier sorts; drop build support for go1.7

.travis.yml

@@ -27,7 +27,7 @@ jobs:
         - codeclimate-test-reporter < coverage.txt
     # YAML alias, for settings shared across the simpler builds
     - &simple-test
-      go: 1.7.x
+      go: 1.9.x
       stage: test
       install: skip
       env:

README.md

@@ -8,7 +8,7 @@
 
 ## Dep
 
-`dep` is a prototype dependency management tool for Go. It requires Go 1.7 or newer to compile.
+`dep` is a prototype dependency management tool for Go. It requires Go 1.8 or newer to compile.
 
 `dep` is the official _experiment_, but not yet the official tool. Check out the [Roadmap](https://github.com/golang/dep/wiki/Roadmap) for more on what this means!
 

cmd/dep/status.go

@@ -349,7 +349,9 @@ func runStatusAll(ctx *dep.Ctx, out outputter, p *dep.Project, sm gps.SourceMana
 	// deterministically ordered. (This may be superfluous if the lock is always
 	// written in alpha order, but it doesn't hurt to double down.)
 	slp := p.Lock.Projects()
-	sort.Sort(dep.SortedLockedProjects(slp))
+	sort.Slice(slp, func(i, j int) bool {
+		return slp[i].Ident().Less(slp[j].Ident())
+	})
 
 	if bytes.Equal(s.HashInputs(), p.Lock.SolveMeta.InputsDigest) {
 		// If these are equal, we're guaranteed that the lock is a transitively

internal/fs/fs.go

@@ -109,7 +109,7 @@ func RenameWithFallback(src, dst string) error {
 		return errors.Wrapf(err, "cannot stat %s", src)
 	}
 
-	err = rename(src, dst)
+	err = os.Rename(src, dst)
 	if err == nil {
 		return nil
 	}

internal/fs/rename.go

@@ -3,7 +3,6 @@
 // license that can be found in the LICENSE file.
 
 // +build !windows
-// +build go1.8
 
 package fs
 
@@ -14,10 +13,6 @@ import (
 	"github.com/pkg/errors"
 )
 
-func rename(src, dst string) error {
-	return os.Rename(src, dst)
-}
-
 // renameFallback attempts to determine the appropriate fallback to failed rename
 // operation depending on the resulting error.
 func renameFallback(err error, src, dst string) error {

internal/fs/rename_windows.go

@@ -13,10 +13,6 @@ import (
 	"github.com/pkg/errors"
 )
 
-func rename(src, dst string) error {
-	return os.Rename(src, dst)
-}
-
 // renameFallback attempts to determine the appropriate fallback to failed rename
 // operation depending on the resulting error.
 func renameFallback(err error, src, dst string) error {

internal/gps/constraints.go

@@ -331,7 +331,9 @@ func (m ProjectConstraints) asSortedSlice() []ProjectConstraint {
 		k++
 	}
 
-	sort.Stable(sortedConstraints(pcs))
+	sort.SliceStable(pcs, func(i, j int) bool {
+		return pcs[i].Ident.Less(pcs[j].Ident)
+	})
 	return pcs
 }
 
@@ -348,7 +350,9 @@ func (m ProjectConstraints) overrideAll(pcm ProjectConstraints) (out []workingCo
 		k++
 	}
 
-	sort.Stable(sortedWC(out))
+	sort.SliceStable(out, func(i, j int) bool {
+		return out[i].Ident.Less(out[j].Ident)
+	})
 	return
 }
 
@@ -389,15 +393,3 @@ func (m ProjectConstraints) override(pr ProjectRoot, pp ProjectProperties) worki
 
 	return wc
 }
-
-type sortedConstraints []ProjectConstraint
-
-func (s sortedConstraints) Len() int           { return len(s) }
-func (s sortedConstraints) Swap(i, j int)      { s[i], s[j] = s[j], s[i] }
-func (s sortedConstraints) Less(i, j int) bool { return s[i].Ident.less(s[j].Ident) }
-
-type sortedWC []workingConstraint
-
-func (s sortedWC) Len() int           { return len(s) }
-func (s sortedWC) Swap(i, j int)      { s[i], s[j] = s[j], s[i] }
-func (s sortedWC) Less(i, j int) bool { return s[i].Ident.less(s[j].Ident) }

internal/gps/identifier.go

@@ -80,14 +80,14 @@ type ProjectIdentifier struct {
 	Source      string
 }
 
-func (i ProjectIdentifier) less(j ProjectIdentifier) bool {
+// Less compares by ProjectRoot then normalized Source.
+func (i ProjectIdentifier) Less(j ProjectIdentifier) bool {
 	if i.ProjectRoot < j.ProjectRoot {
 		return true
 	}
 	if j.ProjectRoot < i.ProjectRoot {
 		return false
 	}
-
 	return i.normalizedSource() < j.normalizedSource()
 }
 

internal/gps/lock.go

@@ -39,20 +39,8 @@ func LocksAreEq(l1, l2 Lock, checkHash bool) bool {
 		return false
 	}
 
-	// Check if the slices are sorted already. If they are, we can compare
-	// without copying. Otherwise, we have to copy to avoid altering the
-	// original input.
-	sp1, sp2 := lpsorter(p1), lpsorter(p2)
-	if len(p1) > 1 && !sort.IsSorted(sp1) {
-		p1 = make([]LockedProject, len(p1))
-		copy(p1, l1.Projects())
-		sort.Sort(lpsorter(p1))
-	}
-	if len(p2) > 1 && !sort.IsSorted(sp2) {
-		p2 = make([]LockedProject, len(p2))
-		copy(p2, l2.Projects())
-		sort.Sort(lpsorter(p2))
-	}
+	p1 = sortedLockedProjects(p1)
+	p2 = sortedLockedProjects(p2)
 
 	for k, lp := range p1 {
 		if !lp.Eq(p2[k]) {
@@ -62,6 +50,21 @@ func LocksAreEq(l1, l2 Lock, checkHash bool) bool {
 	return true
 }
 
+// sortedLockedProjects returns a sorted copy of lps, or itself if already sorted.
+func sortedLockedProjects(lps []LockedProject) []LockedProject {
+	if len(lps) <= 1 || sort.SliceIsSorted(lps, func(i, j int) bool {
+		return lps[i].Ident().Less(lps[j].Ident())
+	}) {
+		return lps
+	}
+	cp := make([]LockedProject, len(lps))
+	copy(cp, lps)
+	sort.Slice(cp, func(i, j int) bool {
+		return cp[i].Ident().Less(cp[j].Ident())
+	})
+	return cp
+}
+
 // LockedProject is a single project entry from a lock file. It expresses the
 // project's name, one or both of version and underlying revision, the network
 // URI for accessing it, the path at which it should be placed within a vendor
@@ -230,23 +233,3 @@ func prepLock(l Lock) safeLock {
 
 	return rl
 }
-
-// SortLockedProjects sorts a slice of LockedProject in alphabetical order by
-// ProjectIdentifier.
-func SortLockedProjects(lps []LockedProject) {
-	sort.Stable(lpsorter(lps))
-}
-
-type lpsorter []LockedProject
-
-func (lps lpsorter) Swap(i, j int) {
-	lps[i], lps[j] = lps[j], lps[i]
-}
-
-func (lps lpsorter) Len() int {
-	return len(lps)
-}
-
-func (lps lpsorter) Less(i, j int) bool {
-	return lps[i].Ident().less(lps[j].Ident())
-}

internal/gps/lock_test.go

@@ -6,6 +6,7 @@ package gps
 
 import (
 	"reflect"
+	"sort"
 	"testing"
 )
 
@@ -20,7 +21,9 @@ func TestLockedProjectSorting(t *testing.T) {
 	lps2 := make([]LockedProject, len(lps))
 	copy(lps2, lps)
 
-	SortLockedProjects(lps2)
+	sort.SliceStable(lps2, func(i, j int) bool {
+		return lps2[i].Ident().Less(lps2[j].Ident())
+	})
 
 	// only the two should have switched positions
 	lps[0], lps[2] = lps[2], lps[0]

internal/gps/lockdiff.go

@@ -74,20 +74,8 @@ func DiffLocks(l1 Lock, l2 Lock) *LockDiff {
 
 	p1, p2 := l1.Projects(), l2.Projects()
 
-	// Check if the slices are sorted already. If they are, we can compare
-	// without copying. Otherwise, we have to copy to avoid altering the
-	// original input.
-	sp1, sp2 := lpsorter(p1), lpsorter(p2)
-	if len(p1) > 1 && !sort.IsSorted(sp1) {
-		p1 = make([]LockedProject, len(p1))
-		copy(p1, l1.Projects())
-		sort.Sort(lpsorter(p1))
-	}
-	if len(p2) > 1 && !sort.IsSorted(sp2) {
-		p2 = make([]LockedProject, len(p2))
-		copy(p2, l2.Projects())
-		sort.Sort(lpsorter(p2))
-	}
+	p1 = sortedLockedProjects(p1)
+	p2 = sortedLockedProjects(p2)
 
 	diff := LockDiff{}
 

internal/gps/manager_test.go

@@ -47,7 +47,7 @@ func mkNaiveSM(t *testing.T) (*SourceMgr, func()) {
 
 	return sm, func() {
 		sm.Release()
-		err := removeAll(cpath)
+		err := os.RemoveAll(cpath)
 		if err != nil {
 			t.Errorf("removeAll failed: %s", err)
 		}
@@ -65,7 +65,7 @@ func remakeNaiveSM(osm *SourceMgr, t *testing.T) (*SourceMgr, func()) {
 
 	return sm, func() {
 		sm.Release()
-		err := removeAll(cpath)
+		err := os.RemoveAll(cpath)
 		if err != nil {
 			t.Errorf("removeAll failed: %s", err)
 		}
@@ -95,7 +95,7 @@ func TestSourceManagerInit(t *testing.T) {
 	}
 
 	sm.Release()
-	err = removeAll(cpath)
+	err = os.RemoveAll(cpath)
 	if err != nil {
 		t.Errorf("removeAll failed: %s", err)
 	}
@@ -111,7 +111,7 @@ func TestSourceManagerInit(t *testing.T) {
 	}
 
 	sm.Release()
-	err = removeAll(cpath)
+	err = os.RemoveAll(cpath)
 	if err != nil {
 		t.Errorf("removeAll failed: %s", err)
 	}
@@ -135,7 +135,7 @@ func TestSourceInit(t *testing.T) {
 
 	defer func() {
 		sm.Release()
-		err := removeAll(cpath)
+		err := os.RemoveAll(cpath)
 		if err != nil {
 			t.Errorf("removeAll failed: %s", err)
 		}