Merge pull request #697 from ibrasho-forks/ensure-packages-are-deducible-before-solving

internal/gps: ensure packages are deducible before attempting to solve

Author: sdboyer

cmd/dep/ensure.go

@@ -209,6 +209,18 @@ func (cmd *ensureCommand) runDefault(ctx *dep.Ctx, args []string, p *dep.Project
 		return errors.New("dep ensure only takes spec arguments with -add or -update")
 	}
 
+	err = gps.ValidateParams(params, sm)
+	if err != nil {
+		if deduceErrs, ok := err.(gps.DeductionErrs); ok {
+			ctx.Err.Println("The following errors occurred while deducing packages:")
+			for ip, dErr := range deduceErrs {
+				ctx.Err.Printf("  * \"%s\": %s", ip, dErr)
+			}
+			ctx.Err.Println()
+		}
+		return errors.Wrap(err, "validateParams")
+	}
+
 	solver, err := gps.Prepare(params, sm)
 	if err != nil {
 		return errors.Wrap(err, "prepare solver")

internal/gps/hash_test.go

@@ -528,7 +528,8 @@ func TestHashInputsOverrides(t *testing.T) {
 
 		s, err := Prepare(params, newdepspecSM(basefix.ds, nil))
 		if err != nil {
-			t.Fatalf("(fix: %q) Unexpected error while prepping solver: %s", fix.name, err)
+			t.Errorf("(fix: %q) Unexpected error while prepping solver: %s", fix.name, err)
+			continue
 		}
 
 		h := sha256.New()

internal/gps/solve_test.go

@@ -7,17 +7,12 @@ package gps
 import (
 	"bytes"
 	"fmt"
-	"io/ioutil"
 	"log"
-	"math/rand"
 	"reflect"
 	"sort"
-	"strconv"
 	"strings"
 	"testing"
 	"unicode"
-
-	"github.com/golang/dep/internal/gps/pkgtree"
 )
 
 // overrideMkBridge overrides the base bridge with the depspecBridge that skips
@@ -274,150 +269,3 @@ func TestRootLockNoVersionPairMatching(t *testing.T) {
 
 	fixtureSolveSimpleChecks(fix, res, err, t)
 }
-
-// TestBadSolveOpts exercises the different possible inputs to a solver that can
-// be determined as invalid in Prepare(), without any further work
-func TestBadSolveOpts(t *testing.T) {
-	pn := strconv.FormatInt(rand.Int63(), 36)
-	fix := basicFixtures["no dependencies"]
-	fix.ds[0].n = ProjectRoot(pn)
-
-	sm := newdepspecSM(fix.ds, nil)
-	params := SolveParameters{
-		mkBridgeFn: overrideMkBridge,
-	}
-
-	_, err := Prepare(params, nil)
-	if err == nil {
-		t.Errorf("Prepare should have errored on nil SourceManager")
-	} else if !strings.Contains(err.Error(), "non-nil SourceManager") {
-		t.Error("Prepare should have given error on nil SourceManager, but gave:", err)
-	}
-
-	_, err = Prepare(params, sm)
-	if err == nil {
-		t.Errorf("Prepare should have errored without ProjectAnalyzer")
-	} else if !strings.Contains(err.Error(), "must provide a ProjectAnalyzer") {
-		t.Error("Prepare should have given error without ProjectAnalyzer, but gave:", err)
-	}
-
-	params.ProjectAnalyzer = naiveAnalyzer{}
-	_, err = Prepare(params, sm)
-	if err == nil {
-		t.Errorf("Prepare should have errored on empty root")
-	} else if !strings.Contains(err.Error(), "non-empty root directory") {
-		t.Error("Prepare should have given error on empty root, but gave:", err)
-	}
-
-	params.RootDir = pn
-	_, err = Prepare(params, sm)
-	if err == nil {
-		t.Errorf("Prepare should have errored on empty name")
-	} else if !strings.Contains(err.Error(), "non-empty import root") {
-		t.Error("Prepare should have given error on empty import root, but gave:", err)
-	}
-
-	params.RootPackageTree = pkgtree.PackageTree{
-		ImportRoot: pn,
-	}
-	_, err = Prepare(params, sm)
-	if err == nil {
-		t.Errorf("Prepare should have errored on empty name")
-	} else if !strings.Contains(err.Error(), "at least one package") {
-		t.Error("Prepare should have given error on empty import root, but gave:", err)
-	}
-
-	params.RootPackageTree = pkgtree.PackageTree{
-		ImportRoot: pn,
-		Packages: map[string]pkgtree.PackageOrErr{
-			pn: {
-				P: pkgtree.Package{
-					ImportPath: pn,
-					Name:       pn,
-				},
-			},
-		},
-	}
-	params.TraceLogger = log.New(ioutil.Discard, "", 0)
-
-	params.Manifest = simpleRootManifest{
-		ovr: ProjectConstraints{
-			ProjectRoot("foo"): ProjectProperties{},
-		},
-	}
-	_, err = Prepare(params, sm)
-	if err == nil {
-		t.Errorf("Should have errored on override with empty ProjectProperties")
-	} else if !strings.Contains(err.Error(), "foo, but without any non-zero properties") {
-		t.Error("Prepare should have given error override with empty ProjectProperties, but gave:", err)
-	}
-
-	params.Manifest = simpleRootManifest{
-		ig:  map[string]bool{"foo": true},
-		req: map[string]bool{"foo": true},
-	}
-	_, err = Prepare(params, sm)
-	if err == nil {
-		t.Errorf("Should have errored on pkg both ignored and required")
-	} else if !strings.Contains(err.Error(), "was given as both a required and ignored package") {
-		t.Error("Prepare should have given error with single ignore/require conflict error, but gave:", err)
-	}
-
-	params.Manifest = simpleRootManifest{
-		ig:  map[string]bool{"foo": true, "bar": true},
-		req: map[string]bool{"foo": true, "bar": true},
-	}
-	_, err = Prepare(params, sm)
-	if err == nil {
-		t.Errorf("Should have errored on pkg both ignored and required")
-	} else if !strings.Contains(err.Error(), "multiple packages given as both required and ignored:") {
-		t.Error("Prepare should have given error with multiple ignore/require conflict error, but gave:", err)
-	}
-	params.Manifest = nil
-
-	params.ToChange = []ProjectRoot{"foo"}
-	_, err = Prepare(params, sm)
-	if err == nil {
-		t.Errorf("Should have errored on non-empty ToChange without a lock provided")
-	} else if !strings.Contains(err.Error(), "update specifically requested for") {
-		t.Error("Prepare should have given error on ToChange without Lock, but gave:", err)
-	}
-
-	params.Lock = safeLock{
-		p: []LockedProject{
-			NewLockedProject(mkPI("bar"), Revision("makebelieve"), nil),
-		},
-	}
-	_, err = Prepare(params, sm)
-	if err == nil {
-		t.Errorf("Should have errored on ToChange containing project not in lock")
-	} else if !strings.Contains(err.Error(), "cannot update foo as it is not in the lock") {
-		t.Error("Prepare should have given error on ToChange with item not present in Lock, but gave:", err)
-	}
-
-	params.Lock, params.ToChange = nil, nil
-	_, err = Prepare(params, sm)
-	if err != nil {
-		t.Error("Basic conditions satisfied, prepare should have completed successfully, err as:", err)
-	}
-
-	// swap out the test mkBridge override temporarily, just to make sure we get
-	// the right error
-	params.mkBridgeFn = nil
-
-	_, err = Prepare(params, sm)
-	if err == nil {
-		t.Errorf("Should have errored on nonexistent root")
-	} else if !strings.Contains(err.Error(), "could not read project root") {
-		t.Error("Prepare should have given error nonexistent project root dir, but gave:", err)
-	}
-
-	// Pointing it at a file should also be an err
-	params.RootDir = "solve_test.go"
-	_, err = Prepare(params, sm)
-	if err == nil {
-		t.Errorf("Should have errored on file for RootDir")
-	} else if !strings.Contains(err.Error(), "is a file, not a directory") {
-		t.Error("Prepare should have given error on file as RootDir, but gave:", err)
-	}
-}

internal/gps/solver.go

@@ -10,6 +10,7 @@ import (
 	"log"
 	"sort"
 	"strings"
+	"sync"
 
 	"github.com/armon/go-radix"
 	"github.com/golang/dep/internal/gps/paths"
@@ -381,6 +382,49 @@ func (s *solver) Version() int {
 	return 1
 }
 
+// DeductionErrs maps package import path to errors occurring during deduction.
+type DeductionErrs map[string]error
+
+func (e DeductionErrs) Error() string {
+	return "could not deduce external imports' project roots"
+}
+
+// ValidateParams validates the solver parameters to ensure solving can be completed.
+func ValidateParams(params SolveParameters, sm SourceManager) error {
+	// Ensure that all packages are deducible without issues.
+	var deducePkgsGroup sync.WaitGroup
+	deductionErrs := make(DeductionErrs)
+	var errsMut sync.Mutex
+
+	rd, err := params.toRootdata()
+	if err != nil {
+		return err
+	}
+
+	deducePkg := func(ip string, sm SourceManager) {
+		_, err := sm.DeduceProjectRoot(ip)
+		if err != nil {
+			errsMut.Lock()
+			deductionErrs[ip] = err
+			errsMut.Unlock()
+		}
+		deducePkgsGroup.Done()
+	}
+
+	for _, ip := range rd.externalImportList(paths.IsStandardImportPath) {
+		deducePkgsGroup.Add(1)
+		go deducePkg(ip, sm)
+	}
+
+	deducePkgsGroup.Wait()
+
+	if len(deductionErrs) > 0 {
+		return deductionErrs
+	}
+
+	return nil
+}
+
 // Solve attempts to find a dependency solution for the given project, as
 // represented by the SolveParameters with which this Solver was created.
 //
@@ -391,8 +435,7 @@ func (s *solver) Solve() (Solution, error) {
 	s.vUnify.mtr = s.mtr
 
 	// Prime the queues with the root project
-	err := s.selectRoot()
-	if err != nil {
+	if err := s.selectRoot(); err != nil {
 		return nil, err
 	}