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_go17.go

@@ -1,50 +0,0 @@
-// Copyright 2016 The Go Authors. All rights reserved.
-// Use of this source code is governed by a BSD-style
-// license that can be found in the LICENSE file.
-
-// +build !windows
-// +build !go1.8
-
-package fs
-
-import (
-	"os"
-	"syscall"
-
-	"github.com/pkg/errors"
-)
-
-func rename(src, dst string) error {
-	fi, err := os.Stat(src)
-	if err != nil {
-		return errors.Wrapf(err, "cannot stat %s", src)
-	}
-
-	// In go 1.8, the behavior of os.Rename changed on non-Windows platforms. It no
-	// longer allows renames that would replace an existing directory. This has
-	// always been the case on Windows, though.
-	//
-	// For consistency, we replicate the go 1.8 behavior in earlier go versions here.
-	if dstfi, err := os.Stat(dst); fi.IsDir() && err == nil && dstfi.IsDir() {
-		return errors.Errorf("cannot rename directory %s to existing dst %s", src, dst)
-	}
-
-	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 {
-	// Rename may fail if src and dst are on different devices; fall back to
-	// copy if we detect that case. syscall.EXDEV is the common name for the
-	// cross device link error which has varying output text across different
-	// operating systems.
-	terr, ok := err.(*os.LinkError)
-	if !ok {
-		return err
-	} else if terr.Err != syscall.EXDEV {
-		return errors.Wrapf(terr, "link error: cannot rename %s to %s", src, dst)
-	}
-
-	return renameByCopy(src, dst)
-}

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)
 		}

internal/gps/remove_go16.go

@@ -1,48 +0,0 @@
-// Copyright 2017 The Go Authors. All rights reserved.
-// Use of this source code is governed by a BSD-style
-// license that can be found in the LICENSE file.
-
-// +build !go1.7
-
-package gps
-
-import (
-	"os"
-	"path/filepath"
-	"runtime"
-)
-
-// removeAll removes path and any children it contains. It deals correctly with
-// removal on Windows where, prior to Go 1.7, there were issues when files were
-// set to read-only.
-func removeAll(path string) error {
-	// Only need special handling for windows
-	if runtime.GOOS != "windows" {
-		return os.RemoveAll(path)
-	}
-
-	// Simple case: if Remove works, we're done.
-	err := os.Remove(path)
-	if err == nil || os.IsNotExist(err) {
-		return nil
-	}
-
-	// make sure all files are writable so we can delete them
-	err = filepath.Walk(path, func(path string, info os.FileInfo, err error) error {
-		if err != nil && err != filepath.SkipDir {
-			// walk gave us some error, give it back.
-			return err
-		}
-		mode := info.Mode()
-		if mode|0200 == mode {
-			return nil
-		}
-
-		return os.Chmod(path, mode|0200)
-	})
-	if err != nil {
-		return err
-	}
-
-	return os.Remove(path)
-}

internal/gps/remove_go17.go

@@ -1,15 +0,0 @@
-// Copyright 2017 The Go Authors. All rights reserved.
-// Use of this source code is governed by a BSD-style
-// license that can be found in the LICENSE file.
-
-// +build go1.7
-
-package gps
-
-import "os"
-
-// go1.7 and later deal with the file perms issue in os.RemoveAll(), so our
-// workaround is no longer necessary.
-func removeAll(path string) error {
-	return os.RemoveAll(path)
-}

internal/gps/result.go

@@ -98,7 +98,7 @@ func WriteDepTree(basedir string, l Lock, sm SourceManager, sv bool, logger *log
 			logger.Println(" * ", err)
 		}
 
-		removeAll(basedir)
+		os.RemoveAll(basedir)
 
 		return errors.New("failed to write dep tree")
 	}

internal/gps/solver.go

@@ -1123,7 +1123,7 @@ func (s *solver) unselectedComparator(i, j int) bool {
 	case !ilock && jlock:
 		return false
 	case ilock && jlock:
-		return iname.less(jname)
+		return iname.Less(jname)
 	}
 
 	// Now, sort by number of available versions. This will trigger network
@@ -1152,7 +1152,7 @@ func (s *solver) unselectedComparator(i, j int) bool {
 	}
 
 	// Finally, if all else fails, fall back to comparing by name
-	return iname.less(jname)
+	return iname.Less(jname)
 }
 
 func (s *solver) fail(id ProjectIdentifier) {

internal/gps/source_test.go

@@ -8,6 +8,7 @@ import (
 	"context"
 	"fmt"
 	"io/ioutil"
+	"os"
 	"reflect"
 	"testing"
 
@@ -30,7 +31,7 @@ func testSourceGateway(t *testing.T) {
 	bgc := context.Background()
 	ctx, cancelFunc := context.WithCancel(bgc)
 	defer func() {
-		removeAll(cachedir)
+		os.RemoveAll(cachedir)
 		cancelFunc()
 	}()
 

internal/gps/strip_vendor.go

@@ -32,7 +32,7 @@ func stripVendor(path string, info os.FileInfo, err error) error {
 		}
 
 		if info.IsDir() {
-			if err := removeAll(path); err != nil {
+			if err := os.RemoveAll(path); err != nil {
 				return err
 			}
 			return filepath.SkipDir

internal/gps/strip_vendor_windows.go

@@ -40,7 +40,7 @@ func stripVendor(path string, info os.FileInfo, err error) error {
 				}
 
 			case dir:
-				if err := removeAll(path); err != nil {
+				if err := os.RemoveAll(path); err != nil {
 					return err
 				}
 				return filepath.SkipDir

internal/gps/vcs_source_test.go

@@ -49,7 +49,7 @@ func testGitSourceInteractions(t *testing.T) {
 		t.Errorf("Failed to create temp dir: %s", err)
 	}
 	defer func() {
-		if err := removeAll(cpath); err != nil {
+		if err := os.RemoveAll(cpath); err != nil {
 			t.Errorf("removeAll failed: %s", err)
 		}
 	}()
@@ -145,7 +145,7 @@ func testGopkginSourceInteractions(t *testing.T) {
 		t.Errorf("Failed to create temp dir: %s", err)
 	}
 	defer func() {
-		if err := removeAll(cpath); err != nil {
+		if err := os.RemoveAll(cpath); err != nil {
 			t.Errorf("removeAll failed: %s", err)
 		}
 	}()
@@ -296,7 +296,7 @@ func testBzrSourceInteractions(t *testing.T) {
 		t.Errorf("Failed to create temp dir: %s", err)
 	}
 	defer func() {
-		if err := removeAll(cpath); err != nil {
+		if err := os.RemoveAll(cpath); err != nil {
 			t.Errorf("removeAll failed: %s", err)
 		}
 	}()
@@ -406,7 +406,7 @@ func testHgSourceInteractions(t *testing.T) {
 		t.Errorf("Failed to create temp dir: %s", err)
 	}
 	defer func() {
-		if err := removeAll(cpath); err != nil {
+		if err := os.RemoveAll(cpath); err != nil {
 			t.Errorf("removeAll failed: %s", err)
 		}
 	}()

lock.go

@@ -149,7 +149,9 @@ func (l *Lock) toRaw() rawLock {
 		Projects: make([]rawLockedProject, len(l.P)),
 	}
 
-	sort.Sort(SortedLockedProjects(l.P))
+	sort.Slice(l.P, func(i, j int) bool {
+		return l.P[i].Ident().Less(l.P[j].Ident())
+	})
 
 	for k, lp := range l.P {
 		id := lp.Ident()
@@ -197,21 +199,3 @@ func LockFromSolution(in gps.Solution) *Lock {
 	copy(l.P, p)
 	return l
 }
-
-// SortedLockedProjects implements sort.Interface.
-type SortedLockedProjects []gps.LockedProject
-
-func (s SortedLockedProjects) Len() int      { return len(s) }
-func (s SortedLockedProjects) Swap(i, j int) { s[i], s[j] = s[j], s[i] }
-func (s SortedLockedProjects) Less(i, j int) bool {
-	l, r := s[i].Ident(), s[j].Ident()
-
-	if l.ProjectRoot < r.ProjectRoot {
-		return true
-	}
-	if r.ProjectRoot < l.ProjectRoot {
-		return false
-	}
-
-	return l.Source < r.Source
-}