Respect dry run flag during dep ensure

cmd/dep/ensure.go

@@ -147,25 +147,27 @@ func (cmd *ensureCommand) Run(ctx *dep.Ctx, args []string) error {
 		return errors.Wrap(err, "ensure Solve()")
 	}
 
-	sw := dep.SafeWriter{
-		Root:          p.AbsRoot,
-		Lock:          p.Lock,
-		NewLock:       solution,
-		SourceManager: sm,
-	}
-	if !cmd.update {
-		sw.Manifest = p.Manifest
-	}
-
 	// check if vendor exists, because if the locks are the same but
 	// vendor does not exist we should write vendor
-	vendorExists, err := dep.IsNonEmptyDir(filepath.Join(sw.Root, "vendor"))
+	vendorExists, err := dep.IsNonEmptyDir(filepath.Join(p.AbsRoot, "vendor"))
 	if err != nil {
 		return errors.Wrap(err, "ensure vendor is a directory")
 	}
 	writeV := !vendorExists && solution != nil
 
-	return errors.Wrap(sw.WriteAllSafe(writeV), "grouped write of manifest, lock and vendor")
+	var sw dep.SafeWriter
+	var manifest *dep.Manifest
+	if !cmd.update {
+		manifest = p.Manifest
+	}
+
+	newLock := dep.LockFromInterface(solution)
+	sw.Prepare(manifest, p.Lock, newLock, writeV)
+	if cmd.dryRun {
+		return sw.PrintPreparedActions()
+	}
+
+	return errors.Wrap(sw.Write(p.AbsRoot, sm), "grouped write of manifest, lock and vendor")
 }
 
 func applyUpdateArgs(args []string, params *gps.SolveParameters) {

cmd/dep/init.go

@@ -106,13 +106,13 @@ func (cmd *initCommand) Run(ctx *dep.Ctx, args []string) error {
 	if err != nil {
 		return err
 	}
-	m := dep.Manifest{
+	m := &dep.Manifest{
 		Dependencies: pd.constraints,
 	}
 
 	// Make an initial lock from what knowledge we've collected about the
 	// versions on disk
-	l := dep.Lock{
+	l := &dep.Lock{
 		P: make([]gps.LockedProject, 0, len(pd.ondisk)),
 	}
 
@@ -134,19 +134,13 @@ func (cmd *initCommand) Run(ctx *dep.Ctx, args []string) error {
 		)
 	}
 
-	sw := dep.SafeWriter{
-		Root:          root,
-		SourceManager: sm,
-		Manifest:      &m,
-	}
-
 	if len(pd.notondisk) > 0 {
 		vlogf("Solving...")
 		params := gps.SolveParameters{
 			RootDir:         root,
 			RootPackageTree: pkgT,
-			Manifest:        &m,
-			Lock:            &l,
+			Manifest:        m,
+			Lock:            l,
 		}
 
 		if *verbose {
@@ -163,14 +157,14 @@ func (cmd *initCommand) Run(ctx *dep.Ctx, args []string) error {
 			handleAllTheFailuresOfTheWorld(err)
 			return err
 		}
-		sw.Lock = dep.LockFromInterface(soln)
-	} else {
-		sw.Lock = &l
+		l = dep.LockFromInterface(soln)
 	}
 
 	vlogf("Writing manifest and lock files.")
 
-	if err := sw.WriteAllSafe(true); err != nil {
+	var sw dep.SafeWriter
+	sw.Prepare(m, l, nil, true)
+	if err := sw.Write(root, sm); err != nil {
 		return errors.Wrap(err, "safe write of manifest and lock")
 	}
 

cmd/dep/remove.go

@@ -179,15 +179,10 @@ func (cmd *removeCommand) Run(ctx *dep.Ctx, args []string) error {
 		return err
 	}
 
-	sw := dep.SafeWriter{
-		Root:          p.AbsRoot,
-		Manifest:      p.Manifest,
-		Lock:          p.Lock,
-		NewLock:       soln,
-		SourceManager: sm,
-	}
-
-	if err := sw.WriteAllSafe(false); err != nil {
+	var sw dep.SafeWriter
+	newLock := dep.LockFromInterface(soln)
+	sw.Prepare(p.Manifest, p.Lock, newLock, false)
+	if err := sw.Write(p.AbsRoot, sm); err != nil {
 		return errors.Wrap(err, "grouped write of manifest, lock and vendor")
 	}
 	return nil

cmd/dep/testdata/harness_tests/ensure/update/case2/final/lock.json

@@ -0,0 +1,21 @@
+{
+    "memo": "9a5243dd3fa20feeaa20398e7283d6c566532e2af1aae279a010df34793761c5",
+    "projects": [
+        {
+            "name": "github.com/sdboyer/deptest",
+            "version": "v0.8.0",
+            "revision": "ff2948a2ac8f538c4ecd55962e919d1e13e74baf",
+            "packages": [
+                "."
+            ]
+        },
+        {
+            "name": "github.com/sdboyer/deptestdos",
+            "version": "v2.0.0",
+            "revision": "5c607206be5decd28e6263ffffdcee067266015e",
+            "packages": [
+                "."
+            ]
+        }
+    ]
+}

cmd/dep/testdata/harness_tests/ensure/update/case2/final/manifest.json

@@ -0,0 +1,7 @@
+{
+    "dependencies": {
+        "github.com/sdboyer/deptest": {
+            "version": "~0.8.0"
+        }
+    }
+}

cmd/dep/testdata/harness_tests/ensure/update/case2/initial/lock.json

@@ -0,0 +1,21 @@
+{
+    "memo": "9a5243dd3fa20feeaa20398e7283d6c566532e2af1aae279a010df34793761c5",
+    "projects": [
+        {
+            "name": "github.com/sdboyer/deptest",
+            "version": "v0.8.0",
+            "revision": "ff2948a2ac8f538c4ecd55962e919d1e13e74baf",
+            "packages": [
+                "."
+            ]
+        },
+        {
+            "name": "github.com/sdboyer/deptestdos",
+            "version": "v2.0.0",
+            "revision": "5c607206be5decd28e6263ffffdcee067266015e",
+            "packages": [
+                "."
+            ]
+        }
+    ]
+}

cmd/dep/testdata/harness_tests/ensure/update/case2/initial/main.go

@@ -0,0 +1,18 @@
+// 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.
+
+package main
+
+import (
+	"github.com/sdboyer/deptest"
+	"github.com/sdboyer/deptestdos"
+)
+
+func main() {
+	err := nil
+	if err != nil {
+		deptest.Map["yo yo!"]
+	}
+	deptestdos.diMeLo("whatev")
+}

cmd/dep/testdata/harness_tests/ensure/update/case2/initial/manifest.json

@@ -0,0 +1,7 @@
+{
+    "dependencies": {
+        "github.com/sdboyer/deptest": {
+            "version": "~0.8.0"
+        }
+    }
+}

cmd/dep/testdata/harness_tests/ensure/update/case2/testcase.json

@@ -0,0 +1,6 @@
+{
+  "commands": [
+    ["init"],
+    ["ensure", "-n", "-update", "github.com/sdboyer/deptest"]
+  ]
+}

fs.go

@@ -6,13 +6,14 @@ package dep
 
 import (
 	"encoding/json"
-	"fmt"
 	"io"
 	"io/ioutil"
 	"os"
 	"path/filepath"
 	"runtime"
 	"syscall"
+
+	"github.com/pkg/errors"
 )
 
 func IsRegular(name string) (bool, error) {
@@ -25,7 +26,7 @@ func IsRegular(name string) (bool, error) {
 		return false, err
 	}
 	if fi.IsDir() {
-		return false, fmt.Errorf("%q is a directory, should be a file", name)
+		return false, errors.Errorf("%q is a directory, should be a file", name)
 	}
 	return true, nil
 }
@@ -40,7 +41,7 @@ func IsDir(name string) (bool, error) {
 		return false, err
 	}
 	if !fi.IsDir() {
-		return false, fmt.Errorf("%q is not a directory", name)
+		return false, errors.Errorf("%q is not a directory", name)
 	}
 	return true, nil
 }
@@ -80,15 +81,15 @@ func writeFile(path string, in json.Marshaler) error {
 func renameWithFallback(src, dest string) error {
 	fi, err := os.Lstat(src)
 	if err != nil {
-		return err
+		return errors.Wrapf(err, "cannot stat %s", src)
 	}
 
 	// Windows cannot use syscall.Rename to rename a directory
 	if runtime.GOOS == "windows" && fi.IsDir() {
 		if err := CopyDir(src, dest); err != nil {
 			return err
 		}
-		return os.RemoveAll(src)
+		return errors.Wrapf(os.RemoveAll(src), "cannot delete %s", src)
 	}
 
 	err = os.Rename(src, dest)
@@ -98,7 +99,7 @@ func renameWithFallback(src, dest string) error {
 
 	terr, ok := err.(*os.LinkError)
 	if !ok {
-		return err
+		return errors.Wrapf(err, "cannot rename %s to %s", src, dest)
 	}
 
 	// Rename may fail if src and dest are on different devices; fall back to
@@ -123,38 +124,38 @@ func renameWithFallback(src, dest string) error {
 			cerr = CopyFile(src, dest)
 		}
 	} else {
-		return terr
+		return errors.Wrapf(terr, "link error: cannot rename %s to %s", src, dest)
 	}
 
 	if cerr != nil {
-		return cerr
+		return errors.Wrapf(cerr, "second attemp failed: cannot rename %s to %s", src, dest)
 	}
 
-	return os.RemoveAll(src)
+	return errors.Wrapf(os.RemoveAll(src), "cannot delete %s", src)
 }
 
 // CopyDir takes in a directory and copies its contents to the destination.
 // It preserves the file mode on files as well.
 func CopyDir(src string, dest string) error {
 	fi, err := os.Lstat(src)
 	if err != nil {
-		return err
+		return errors.Wrapf(err, "cannot stat %s", src)
 	}
 
 	err = os.MkdirAll(dest, fi.Mode())
 	if err != nil {
-		return err
+		return errors.Wrapf(err, "cannot mkdir %s", dest)
 	}
 
 	dir, err := os.Open(src)
 	if err != nil {
-		return err
+		return errors.Wrapf(err, "cannot open %s", src)
 	}
 	defer dir.Close()
 
 	objects, err := dir.Readdir(-1)
 	if err != nil {
-		return err
+		return errors.Wrapf(err, "cannot read directory %s", dir.Name())
 	}
 
 	for _, obj := range objects {

lock.go

@@ -102,23 +102,7 @@ func (l *Lock) MarshalJSON() ([]byte, error) {
 		}
 
 		v := lp.Version()
-		// Figure out how to get the underlying revision
-		switch tv := v.(type) {
-		case gps.UnpairedVersion:
-			// TODO we could error here, if we want to be very defensive about not
-			// allowing a lock to be written if without an immmutable revision
-		case gps.Revision:
-			ld.Revision = tv.String()
-		case gps.PairedVersion:
-			ld.Revision = tv.Underlying().String()
-		}
-
-		switch v.Type() {
-		case gps.IsBranch:
-			ld.Branch = v.String()
-		case gps.IsSemver, gps.IsVersion:
-			ld.Version = v.String()
-		}
+		ld.Revision, ld.Branch, ld.Version = getVersionInfo(v)
 
 		raw.P[k] = ld
 	}
@@ -134,6 +118,29 @@ func (l *Lock) MarshalJSON() ([]byte, error) {
 	return buf.Bytes(), err
 }
 
+// TODO(carolynvs) this should be moved to gps
+func getVersionInfo(v gps.Version) (revision string, branch string, version string) {
+	// Figure out how to get the underlying revision
+	switch tv := v.(type) {
+	case gps.UnpairedVersion:
+	// TODO we could error here, if we want to be very defensive about not
+	// allowing a lock to be written if without an immmutable revision
+	case gps.Revision:
+		revision = tv.String()
+	case gps.PairedVersion:
+		revision = tv.Underlying().String()
+	}
+
+	switch v.Type() {
+	case gps.IsBranch:
+		branch = v.String()
+	case gps.IsSemver, gps.IsVersion:
+		version = v.String()
+	}
+
+	return
+}
+
 // LockFromInterface converts an arbitrary gps.Lock to dep's representation of a
 // lock. If the input is already dep's *lock, the input is returned directly.
 //
@@ -177,14 +184,3 @@ func (s SortedLockedProjects) Less(i, j int) bool {
 
 	return l.Source < r.Source
 }
-
-// locksAreEquivalent compares two locks to see if they differ. If EITHER lock
-// is nil, or their memos do not match, or any projects differ, then false is
-// returned.
-func locksAreEquivalent(l, r *Lock) bool {
-	if l == nil || r == nil {
-		return false
-	}
-
-	return gps.LocksAreEq(l, r, true)
-}