Change up how download/restore works a little

* Try to load the package after downloading/restoring. Previously
  that was done too early in the process.
* make previous verbose output debug output
* report a typed error instead of a string from listPackage so it can
  be asserted to provide a nicer error.
* Catch go get errors that say there are no go files found. See code
  comment as to why.
* do *all* downloading during download phase.

Changelog.md

@@ -1,3 +1,15 @@
+# v37 2015/12/15
+
+* Change up how download/restore works a little
+    * Try to load the package after downloading/restoring. Previously
+      that was done too early in the process.
+    * make previous verbose output debug output
+    * report a typed error instead of a string from listPackage so it can
+      be asserted to provide a nicer error.
+    * Catch go get errors that say there are no go files found. See code
+      comment as to why.
+    * do *all* downloading during download phase.
+
 # v36 2015/12/14
 
 * Fixes #358: Using wrong variable. Will add test after release.

list.go

@@ -1,7 +1,6 @@
 package main
 
 import (
-	"errors"
 	"fmt"
 	"go/build"
 	"go/parser"
@@ -20,6 +19,14 @@ var (
 	gorootSrc = filepath.Join(build.Default.GOROOT, "src")
 )
 
+type errorMissingDep struct {
+	i, dir string // import, dir
+}
+
+func (e errorMissingDep) Error() string {
+	return "Unable to find dependent package " + e.i + " in context of " + e.dir
+}
+
 // packageContext is used to track an import and which package imported it.
 type packageContext struct {
 	pkg *build.Package // package that imports the import
@@ -167,13 +174,13 @@ func listPackage(path string) (*Package, error) {
 		dp, err = build.Import(i, ip.Dir, build.FindOnly)
 		if err != nil {
 			ppln(err)
-			return nil, errors.New("Unable to find dependent package " + i + " in context of " + ip.Dir)
+			return nil, errorMissingDep{i: i, dir: ip.Dir}
 		}
 	Found:
 		dp, err = fullPackageInDir(dp.Dir)
 		if err != nil { // This really should happen in this context though
 			ppln(err)
-			return nil, errors.New("Unable to find dependent package " + i + " in context of " + ip.Dir)
+			return nil, errorMissingDep{i: i, dir: ip.Dir}
 		}
 		ppln(dp)
 		if !dp.Goroot {

msg.go

@@ -2,6 +2,7 @@ package main
 
 import (
 	"fmt"
+	"log"
 
 	"github.com/tools/godep/Godeps/_workspace/src/github.com/kr/pretty"
 )
@@ -13,13 +14,25 @@ func debugln(a ...interface{}) (int, error) {
 	return 0, nil
 }
 
+func verboseln(a ...interface{}) {
+	if verbose {
+		log.Println(a...)
+	}
+}
+
 func debugf(format string, a ...interface{}) (int, error) {
 	if debug {
 		return fmt.Printf(format, a...)
 	}
 	return 0, nil
 }
 
+func verbosef(format string, a ...interface{}) {
+	if verbose {
+		log.Printf(format, a...)
+	}
+}
+
 func pp(a ...interface{}) (int, error) {
 	if debug {
 		return pretty.Print(a...)

restore.go

@@ -1,8 +1,10 @@
 package main
 
 import (
+	"go/build"
 	"log"
 	"os"
+	"strings"
 )
 
 var cmdRestore = &Command{
@@ -18,52 +20,81 @@ If -d is given, debug output is enabled (you probably don't want this, see -v ab
 	Run: runRestore,
 }
 
+// Three phases:
+// 1. Download all deps
+// 2. Restore all deps (checkout the recorded rev)
+// 3. Attempt to load all deps as a simple consistency check
 func runRestore(cmd *Command, args []string) {
+	var hadError bool
+	checkErr := func() {
+		if hadError {
+			os.Exit(1)
+		}
+	}
+
 	g, err := loadDefaultGodepsFile()
 	if err != nil {
 		log.Fatalln(err)
 	}
-	hadError := false
+	for i, dep := range g.Deps {
+		verboseln("Downloading dependency (if needed):", dep.ImportPath)
+		err := download(&dep)
+		if err != nil {
+			log.Printf("error downloading dep (%s): %s\n", dep.ImportPath, err)
+			hadError = true
+		}
+		g.Deps[i] = dep
+	}
+	checkErr()
 	for _, dep := range g.Deps {
-		err := download(dep)
+		verboseln("Restoring dependency (if needed):", dep.ImportPath)
+		err := restore(dep)
 		if err != nil {
-			log.Printf("error downloading dep (%s): %s\n", dep.ImportPath, err.Error())
+			log.Printf("error restoring dep (%s): %s\n", dep.ImportPath, err)
 			hadError = true
 		}
 	}
-	if !hadError {
-		for _, dep := range g.Deps {
-			err := restore(dep)
-			if err != nil {
-				log.Printf("error restoring dep (%s): %s\n", dep.ImportPath, err.Error())
-				hadError = true
+	checkErr()
+	for _, dep := range g.Deps {
+		verboseln("Checking dependency:", dep.ImportPath)
+		_, err := LoadPackages(dep.ImportPath)
+		if err != nil {
+			log.Printf("Dep (%s) restored, but was unable to load it with error:\n\t%s\n", dep.ImportPath, err)
+			if me, ok := err.(errorMissingDep); ok {
+				log.Println("\tThis may be because the dependencies were saved with an older version of godep (< v33).")
+				log.Printf("\tTry `go get %s`. Then `godep save` to update deps.\n", me.i)
 			}
+			hadError = true
 		}
 	}
-	if hadError {
-		os.Exit(1)
-	}
+	checkErr()
 }
 
 // download downloads the given dependency.
-func download(dep Dependency) error {
+// 2 Passes: 1) go get -d <pkg>, 2) git pull (if necessary)
+func download(dep *Dependency) error {
 	// make sure pkg exists somewhere in GOPATH
 
 	args := []string{"get", "-d"}
-	if verbose {
+	if debug {
 		args = append(args, "-v")
 	}
 
-	return runIn(".", "go", append(args, dep.ImportPath)...)
-}
+	o, err := runInWithOutput(".", "go", append(args, dep.ImportPath)...)
+	if strings.Contains(o, "no buildable Go source files") {
+		// We were able to fetch the repo, but didn't find any code to build
+		// this can happen when a repo has changed structure or if the dep won't normally
+		// be built on the current architecture until we implement our own fetcher this
+		// may be the "best"" we can do.
+		// TODO: replace go get
+		err = nil
+	}
 
-// restore checks out the given revision.
-func restore(dep Dependency) error {
-	ps, err := LoadPackages(dep.ImportPath)
+	pkg, err := build.Import(dep.ImportPath, ".", build.FindOnly)
 	if err != nil {
+		debugln("Error finding package "+dep.ImportPath+" after go get:", err)
 		return err
 	}
-	pkg := ps[0]
 
 	dep.vcs, err = VCSForImportPath(dep.ImportPath)
 	if err != nil {
@@ -76,5 +107,20 @@ func restore(dep Dependency) error {
 	if !dep.vcs.exists(pkg.Dir, dep.Rev) {
 		dep.vcs.vcs.Download(pkg.Dir)
 	}
+
+	return nil
+}
+
+// restore checks out the given revision.
+func restore(dep Dependency) error {
+	debugln("Restoring:", dep.ImportPath, dep.Rev)
+
+	pkg, err := build.Import(dep.ImportPath, ".", build.FindOnly)
+	if err != nil {
+		// THi should never happen
+		debugln("Error finding package "+dep.ImportPath+" on restore:", err)
+		return err
+	}
+
 	return dep.vcs.RevSync(pkg.Dir, dep.Rev)
 }

util.go

@@ -10,14 +10,19 @@ import (
 // Stdout, stderr, and the environment are inherited
 // from the current process.
 func runIn(dir, name string, args ...string) error {
+	_, err := runInWithOutput(dir, name, args...)
+	return err
+}
+
+func runInWithOutput(dir, name string, args ...string) (string, error) {
 	c := exec.Command(name, args...)
 	c.Dir = dir
-	output, err := c.CombinedOutput()
+	o, err := c.CombinedOutput()
 
-	if verbose {
+	if debug {
 		fmt.Printf("execute: %+v\n", c)
-		fmt.Printf(" output: %s\n", output)
+		fmt.Printf(" output: %s\n", string(o))
 	}
 
-	return err
+	return string(o), err
 }

vcs.go

@@ -80,7 +80,7 @@ func VCSFromDir(dir, srcRoot string) (*VCS, string, error) {
 
 // VCSForImportPath returns a VCS value for an import path.
 func VCSForImportPath(importPath string) (*VCS, error) {
-	rr, err := vcs.RepoRootForImportPath(importPath, verbose)
+	rr, err := vcs.RepoRootForImportPath(importPath, debug)
 	if err != nil {
 		return nil, err
 	}

version.go

@@ -5,7 +5,7 @@ import (
 	"runtime"
 )
 
-const version = 36
+const version = 37
 
 var cmdVersion = &Command{
 	Name:  "version",