dep: Make noverify preserve excess vendor paths

This helps handle the not-uncommon case that you might need a metadata
file or two in your vendor directory, and you want dep to ignore its
existence.

Author: sdboyer

cmd/dep/check.go

@@ -141,62 +141,83 @@ func (cmd *checkCommand) Run(ctx *dep.Ctx, args []string) error {
 			noverify[skip] = true
 		}
 
-		var vendorfail bool
+		var vendorfail, hasnoverify bool
 		// One full pass through, to see if we need to print the header, and to
 		// create an array of names to sort for deterministic output.
 		var ordered []string
 		for path, status := range statuses {
 			ordered = append(ordered, path)
 
 			switch status {
-			case verify.DigestMismatchInLock, verify.HashVersionMismatch, verify.EmptyDigestInLock:
-				// NoVerify applies only to these three cases.
+			case verify.DigestMismatchInLock, verify.HashVersionMismatch, verify.EmptyDigestInLock, verify.NotInLock:
 				if noverify[path] {
+					hasnoverify = true
 					continue
 				}
 				fallthrough
-			case verify.NotInTree, verify.NotInLock:
+			case verify.NotInTree:
+				// NoVerify cannot be used to make dep check ignore the absence
+				// of a project entirely.
+				if noverify[path] {
+					delete(noverify, path)
+				}
+
 				fail = true
 				if !vendorfail {
 					vendorfail = true
-					logger.Println("# vendor is out of sync:")
 				}
-
 			}
 		}
 		sort.Strings(ordered)
 
+		var vfbuf, novbuf bytes.Buffer
+		var bufptr *bytes.Buffer
+
+		fmt.Fprintf(&vfbuf, "# vendor is out of sync:\n")
+		fmt.Fprintf(&novbuf, "# out of sync, but ignored, due to noverify in Gopkg.toml:\n")
+
 		for _, pr := range ordered {
-			var nvSuffix string
 			if noverify[pr] {
-				nvSuffix = "  (CHECK IGNORED: marked noverify in Gopkg.toml)"
+				bufptr = &novbuf
+			} else {
+				bufptr = &vfbuf
 			}
 
 			status := statuses[pr]
 			switch status {
 			case verify.NotInTree:
-				logger.Printf("%s: missing from vendor\n", pr)
+				fmt.Fprintf(bufptr, "%s: missing from vendor\n", pr)
 			case verify.NotInLock:
 				fi, err := os.Stat(filepath.Join(p.AbsRoot, "vendor", pr))
 				if err != nil {
 					return errors.Wrap(err, "could not stat file that VerifyVendor claimed existed")
 				}
 				if fi.IsDir() {
-					logger.Printf("%s: unused project\n", pr)
+					fmt.Fprintf(bufptr, "%s: unused project\n", pr)
 				} else {
-					logger.Printf("%s: orphaned file\n", pr)
+					fmt.Fprintf(bufptr, "%s: orphaned file\n", pr)
 				}
 			case verify.DigestMismatchInLock:
-				logger.Printf("%s: hash of vendored tree not equal to digest in Gopkg.lock%s\n", pr, nvSuffix)
+				fmt.Fprintf(bufptr, "%s: hash of vendored tree not equal to digest in Gopkg.lock\n", pr)
 			case verify.EmptyDigestInLock:
-				logger.Printf("%s: no digest in Gopkg.lock to compare against hash of vendored tree%s\n", pr, nvSuffix)
+				fmt.Fprintf(bufptr, "%s: no digest in Gopkg.lock to compare against hash of vendored tree\n", pr)
 			case verify.HashVersionMismatch:
 				// This will double-print if the hash version is zero, but
 				// that's a rare case that really only occurs before the first
 				// run with a version of dep >=0.5.0, so it's fine.
-				logger.Printf("%s: hash algorithm mismatch, want version %v%s\n", pr, verify.HashVersion, nvSuffix)
+				fmt.Fprintf(bufptr, "%s: hash algorithm mismatch, want version %v\n", pr, verify.HashVersion)
 			}
 		}
+
+		if vendorfail {
+			logger.Print(vfbuf.String())
+			if hasnoverify {
+				logger.Println()
+			}
+		}
+		if hasnoverify {
+			logger.Print(novbuf.String())
+		}
 	}
 
 	if fail {

cmd/dep/testdata/harness_tests/check/noverify/hash_mismatch/stdout.txt

@@ -1 +1,2 @@
-github.com/sdboyer/deptest: hash of vendored tree not equal to digest in Gopkg.lock  (CHECK IGNORED: marked noverify in Gopkg.toml)
+# out of sync, but ignored, due to noverify in Gopkg.toml:
+github.com/sdboyer/deptest: hash of vendored tree not equal to digest in Gopkg.lock

cmd/dep/testdata/harness_tests/check/noverify/hash_version_mismatch/stdout.txt

@@ -1 +1,2 @@
-github.com/sdboyer/deptest: hash algorithm mismatch, want version 1  (CHECK IGNORED: marked noverify in Gopkg.toml)
+# out of sync, but ignored, due to noverify in Gopkg.toml:
+github.com/sdboyer/deptest: hash algorithm mismatch, want version 1

txn_writer.go

@@ -432,6 +432,7 @@ const (
 	missingFromTree
 	projectAdded
 	projectRemoved
+	pathPreserved
 )
 
 // NewDeltaWriter prepares a vendor writer that will construct a vendor
@@ -510,17 +511,26 @@ func NewDeltaWriter(p *Project, newLock *Lock, behavior VendorBehavior) (TreeWri
 		// We don't validate this field elsewhere as it can be difficult to know
 		// at the beginning of a dep ensure command whether or not the noverify
 		// project actually will exist as part of the Lock by the end of the
-		// run. So, only apply if it's in the lockdiff, and isn't a removal.
+		// run. So, only apply if it's in the lockdiff.
 		if _, has := dw.lockDiff.ProjectDeltas[pr]; has {
-			if typ, has := dw.changed[pr]; has && typ < noVerify {
-				// Avoid writing noverify projects at all for the lower change
-				// types.
-				delete(dw.changed, pr)
-
-				// Uncomment this if we want to switch to the safer behavior,
-				// where we ALWAYS write noverify projects.
-				//dw.changed[pr] = noVerify
+			if typ, has := dw.changed[pr]; has {
+				if typ < noVerify {
+					// Avoid writing noverify projects at all for the lower change
+					// types.
+					delete(dw.changed, pr)
+
+					// Uncomment this if we want to switch to the safer behavior,
+					// where we ALWAYS write noverify projects.
+					//dw.changed[pr] = noVerify
+				} else if typ == projectRemoved {
+					// noverify can also be used to preserve files that would
+					// otherwise be removed.
+					dw.changed[pr] = pathPreserved
+				}
 			}
+			// It's also allowed to preserve entirely unknown paths using noverify.
+		} else if _, has := status[spr]; has {
+			dw.changed[pr] = pathPreserved
 		}
 	}
 
@@ -564,29 +574,28 @@ func (dw *DeltaWriter) Write(path string, sm gps.SourceManager, examples bool, l
 		projs[lp.Ident().ProjectRoot] = lp
 	}
 
-	dropped := []gps.ProjectRoot{}
+	var dropped, preserved []gps.ProjectRoot
 	i := 0
 	tot := len(dw.changed)
-	if len(dw.changed) > 0 {
-		logger.Println("# Bringing vendor into sync")
+	for _, reason := range dw.changed {
+		if reason != pathPreserved {
+			logger.Println("# Bringing vendor into sync")
+			break
+		}
 	}
+
 	for pr, reason := range dw.changed {
-		if reason == projectRemoved {
+		switch reason {
+		case projectRemoved:
 			dropped = append(dropped, pr)
 			continue
+		case pathPreserved:
+			preserved = append(preserved, pr)
+			continue
 		}
 
 		to := filepath.FromSlash(filepath.Join(vnewpath, string(pr)))
-		proj, has := projs[pr]
-		if !has {
-			// This shouldn't be reachable, but it's preferable to print an
-			// error and continue rather than panic. https://github.com/golang/dep/issues/1945
-			// TODO(sdboyer) remove this once we've increased confidence around
-			// this case.
-			fmt.Fprintf(os.Stderr, "Internal error - %s had change code %v but was not in new Gopkg.lock. Re-running dep ensure should fix this. Please file a bug at https://github.com/golang/dep/issues/new!\n", pr, reason)
-			continue
-		}
-		po := proj.(verify.VerifiableProject).PruneOpts
+		po := projs[pr].(verify.VerifiableProject).PruneOpts
 		if err := sm.ExportPrunedProject(context.TODO(), projs[pr], po, to); err != nil {
 			return errors.Wrapf(err, "failed to export %s", pr)
 		}
@@ -666,13 +675,18 @@ func (dw *DeltaWriter) Write(path string, sm gps.SourceManager, examples bool, l
 		}
 	}
 
-	// Ensure vendor/.git is preserved if present
+	// Special case: ensure vendor/.git is preserved if present
 	if hasDotGit(vpath) {
-		err = fs.RenameWithFallback(filepath.Join(vpath, ".git"), filepath.Join(vnewpath, ".git"))
+		preserved = append(preserved, ".git")
+	}
+
+	for _, path := range preserved {
+		err = fs.RenameWithFallback(filepath.Join(vpath, string(path)), filepath.Join(vnewpath, string(path)))
 		if err != nil {
-			return errors.Wrap(err, "failed to preserve vendor/.git")
+			return errors.Wrapf(err, "failed to preserve vendor/%s", path)
 		}
 	}
+
 	err = os.RemoveAll(vpath)
 	if err != nil {
 		return errors.Wrap(err, "failed to remove original vendor directory")