Move the payload into SafeWriter

Author: carolynvs

cmd/dep/ensure.go

@@ -147,14 +147,6 @@ func (cmd *ensureCommand) Run(ctx *dep.Ctx, args []string) error {
 		return errors.Wrap(err, "ensure Solve()")
 	}
 
-	sw := dep.SafeWriter{
-		Lock:    p.Lock,
-		NewLock: solution,
-	}
-	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(p.AbsRoot, "vendor"))
@@ -163,12 +155,18 @@ func (cmd *ensureCommand) Run(ctx *dep.Ctx, args []string) error {
 	}
 	writeV := !vendorExists && solution != nil
 
-	payload := sw.Prepare(writeV)
+	var sw dep.SafeWriter
+	var manifest *dep.Manifest
+	if !cmd.update {
+		manifest = p.Manifest
+	}
+
+	sw.Prepare(manifest, p.Lock, solution, writeV)
 	if cmd.dryRun {
-		return payload.PrintPreparedActions()
+		return sw.PrintPreparedActions()
 	}
 
-	return errors.Wrap(payload.Write(p.AbsRoot, sm), "grouped write of manifest, lock and vendor")
+	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,17 +134,13 @@ func (cmd *initCommand) Run(ctx *dep.Ctx, args []string) error {
 		)
 	}
 
-	sw := dep.SafeWriter{
-		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 {
@@ -161,15 +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.")
 
-	payload := sw.Prepare(true)
-	if err := payload.Write(root, sm); 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,14 +179,9 @@ func (cmd *removeCommand) Run(ctx *dep.Ctx, args []string) error {
 		return err
 	}
 
-	sw := dep.SafeWriter{
-		Manifest: p.Manifest,
-		Lock:     p.Lock,
-		NewLock:  soln,
-	}
-
-	payload := sw.Prepare(false)
-	if err := payload.Write(p.AbsRoot, sm); err != nil {
+	var sw dep.SafeWriter
+	sw.Prepare(p.Manifest, p.Lock, soln, false)
+	if err := sw.Write(p.AbsRoot, sm); err != nil {
 		return errors.Wrap(err, "grouped write of manifest, lock and vendor")
 	}
 	return nil

txn_writer.go

@@ -21,9 +21,7 @@ import (
 // It is not impervious to errors (writing to disk is hard), but it should
 // guard against non-arcane failure conditions.
 type SafeWriter struct {
-	Manifest *Manifest // the manifest to write, if any
-	Lock     *Lock     // the old lock, if any
-	NewLock  gps.Lock  // the new lock, if any
+	Payload SafeWriterPayload
 }
 
 // SafeWriterPayload represents the actions SafeWriter will execute when SafeWriter.Write is called.
@@ -66,44 +64,37 @@ type LockedProjectDiff struct {
 
 // Prepare to write a set of config yaml, lock and vendor tree.
 //
-// - If sw.Manifest is provided, it will be written to the standard manifest file
+// - If manifest is provided, it will be written to the standard manifest file
 //   name beneath root.
-// - If sw.Lock is provided it will be written to the standard
+// - If lock is provided it will be written to the standard
 //   lock file name in the root dir, but vendor will NOT be written
-// - If sw.Lock and sw.NewLock are both provided and are equivalent, then neither lock
+// - If lock and newLock are both provided and are equivalent, then neither lock
 //   nor vendor will be written
-// - If sw.Lock and sw.NewLock are both provided and are not equivalent,
-//   the nl will be written to the same location as above, and a vendor
-//   tree will be written to sw.Root/vendor
-// - If sw.NewLock is provided and sw.Lockock is not, it will write both a lock
-//   and vendor dir in the same way
-// - If the forceVendor param is true, then vendor will be unconditionally
-//   written out based on sw.NewLock if present, else sw.Lock, else error.
-func (sw SafeWriter) Prepare(forceVendor bool) SafeWriterPayload {
-	// Decide which writes we need to do
-	var payload SafeWriterPayload
-	payload.ForceWriteVendor = forceVendor
-
-	if sw.Manifest != nil {
-		payload.Manifest = sw.Manifest
-	}
-
-	if sw.NewLock != nil {
-		rlf := LockFromInterface(sw.NewLock)
-		if sw.Lock == nil {
-			payload.Lock = rlf
-			payload.ForceWriteVendor = true
+// - If lock and newLock are both provided and are not equivalent,
+//   the newLock will be written to the same location as above, and a vendor
+//   tree will be written to vendor/
+// - If newLock is provided and lock is not, it will write both a lock
+//   and vendor/ dir in the same way
+// - If the forceVendor param is true, then vendor/ will be unconditionally
+//   written out based on newLock if present, else lock, else error.
+func (sw *SafeWriter) Prepare(manifest *Manifest, lock *Lock, newLock gps.Lock, forceVendor bool) {
+	sw.Payload.Manifest = manifest
+	sw.Payload.ForceWriteVendor = forceVendor
+
+	if newLock != nil {
+		rlf := LockFromInterface(newLock)
+		if lock == nil {
+			sw.Payload.Lock = rlf
+			sw.Payload.ForceWriteVendor = true
 		} else {
-			if !locksAreEquivalent(rlf, sw.Lock) {
-				payload.Lock = rlf
-				payload.ForceWriteVendor = true
+			if !locksAreEquivalent(rlf, lock) {
+				sw.Payload.Lock = rlf
+				sw.Payload.ForceWriteVendor = true
 			}
 		}
-	} else if sw.Lock != nil {
-		payload.Lock = sw.Lock
+	} else if lock != nil {
+		sw.Payload.Lock = lock
 	}
-
-	return payload
 }
 
 func (payload SafeWriterPayload) validate(root string, sm gps.SourceManager) error {
@@ -136,13 +127,13 @@ func (payload SafeWriterPayload) validate(root string, sm gps.SourceManager) err
 // operations succeeded. It also does its best to roll back if any moves fail.
 // This mostly guarantees that dep cannot exit with a partial write that would
 // leave an undefined state on disk.
-func (payload SafeWriterPayload) Write(root string, sm gps.SourceManager) error {
-	err := payload.validate(root, sm)
+func (sw *SafeWriter) Write(root string, sm gps.SourceManager) error {
+	err := sw.Payload.validate(root, sm)
 	if err != nil {
 		return err
 	}
 
-	if !payload.ShouldWriteManifest() && !payload.ShouldWriteLock() && !payload.ShouldWriteVendor() {
+	if !sw.Payload.ShouldWriteManifest() && !sw.Payload.ShouldWriteLock() && !sw.Payload.ShouldWriteVendor() {
 		// nothing to do
 		return nil
 	}
@@ -157,20 +148,20 @@ func (payload SafeWriterPayload) Write(root string, sm gps.SourceManager) error
 	}
 	defer os.RemoveAll(td)
 
-	if payload.ShouldWriteManifest() {
-		if err := writeFile(filepath.Join(td, ManifestName), payload.Manifest); err != nil {
+	if sw.Payload.ShouldWriteManifest() {
+		if err := writeFile(filepath.Join(td, ManifestName), sw.Payload.Manifest); err != nil {
 			return errors.Wrap(err, "failed to write manifest file to temp dir")
 		}
 	}
 
-	if payload.ShouldWriteLock() {
-		if err := writeFile(filepath.Join(td, LockName), payload.Lock); err != nil {
+	if sw.Payload.ShouldWriteLock() {
+		if err := writeFile(filepath.Join(td, LockName), sw.Payload.Lock); err != nil {
 			return errors.Wrap(err, "failed to write lock file to temp dir")
 		}
 	}
 
-	if payload.ShouldWriteVendor() {
-		err = gps.WriteDepTree(filepath.Join(td, "vendor"), payload.Lock, sm, true)
+	if sw.Payload.ShouldWriteVendor() {
+		err = gps.WriteDepTree(filepath.Join(td, "vendor"), sw.Payload.Lock, sm, true)
 		if err != nil {
 			return errors.Wrap(err, "error while writing out vendor tree")
 		}
@@ -185,7 +176,7 @@ func (payload SafeWriterPayload) Write(root string, sm gps.SourceManager) error
 	var failerr error
 	var vendorbak string
 
-	if payload.ShouldWriteManifest() {
+	if sw.Payload.ShouldWriteManifest() {
 		if _, err := os.Stat(mpath); err == nil {
 			// Move out the old one.
 			tmploc := filepath.Join(td, ManifestName+".orig")
@@ -203,7 +194,7 @@ func (payload SafeWriterPayload) Write(root string, sm gps.SourceManager) error
 		}
 	}
 
-	if payload.ShouldWriteLock() {
+	if sw.Payload.ShouldWriteLock() {
 		if _, err := os.Stat(lpath); err == nil {
 			// Move out the old one.
 			tmploc := filepath.Join(td, LockName+".orig")
@@ -222,7 +213,7 @@ func (payload SafeWriterPayload) Write(root string, sm gps.SourceManager) error
 		}
 	}
 
-	if payload.ShouldWriteVendor() {
+	if sw.Payload.ShouldWriteVendor() {
 		if _, err := os.Stat(vpath); err == nil {
 			// Move out the old vendor dir. just do it into an adjacent dir, to
 			// try to mitigate the possibility of a pointless cross-filesystem
@@ -250,7 +241,7 @@ func (payload SafeWriterPayload) Write(root string, sm gps.SourceManager) error
 
 	// Renames all went smoothly. The deferred os.RemoveAll will get the temp
 	// dir, but if we wrote vendor, we have to clean that up directly
-	if payload.ShouldWriteVendor() {
+	if sw.Payload.ShouldWriteVendor() {
 		// Nothing we can really do about an error at this point, so ignore it
 		os.RemoveAll(vendorbak)
 	}
@@ -266,28 +257,28 @@ fail:
 	return failerr
 }
 
-func (payload SafeWriterPayload) PrintPreparedActions() error {
-	if payload.ShouldWriteManifest() {
+func (sw *SafeWriter) PrintPreparedActions() error {
+	if sw.Payload.ShouldWriteManifest() {
 		fmt.Println("Would have written the following manifest.json:")
-		m, err := payload.Manifest.MarshalJSON()
+		m, err := sw.Payload.Manifest.MarshalJSON()
 		if err != nil {
 			return errors.Wrap(err, "ensure DryRun cannot read manifest")
 		}
 		fmt.Println(string(m))
 	}
 
-	if payload.ShouldWriteLock() {
+	if sw.Payload.ShouldWriteLock() {
 		fmt.Println("Would have written the following lock.json:")
-		m, err := payload.Lock.MarshalJSON()
+		m, err := sw.Payload.Lock.MarshalJSON()
 		if err != nil {
 			return errors.Wrap(err, "ensure DryRun cannot read lock")
 		}
 		fmt.Println(string(m))
 	}
 
-	if payload.ShouldWriteVendor() {
+	if sw.Payload.ShouldWriteVendor() {
 		fmt.Println("Would have written the following projects to the vendor directory:")
-		for _, project := range payload.Lock.Projects() {
+		for _, project := range sw.Payload.Lock.Projects() {
 			prj := project.Ident()
 			rev := GetRevisionFromVersion(project.Version())
 			if prj.Source == "" {