cmd/dep: make checkErrors more lenient (#997)

checkErrors doesn't allow dep ensure to be used in a project that
contains any Go files containing build errors. This commit update
checkErrors to return a warning instead of an error as long as the
project contains some compilable Go files.

Fixes #995

Signed-off-by: Ibrahim AshShohail <[email protected]>

Author: ibrasho

cmd/dep/ensure.go

@@ -174,8 +174,12 @@ func (cmd *ensureCommand) Run(ctx *dep.Ctx, args []string) error {
 		return errors.Wrap(err, "ensure ListPackage for project")
 	}
 
-	if err := checkErrors(params.RootPackageTree.Packages); err != nil {
-		return err
+	if fatal, err := checkErrors(params.RootPackageTree.Packages); err != nil {
+		if fatal {
+			return err
+		} else if ctx.Verbose {
+			ctx.Out.Println(err)
+		}
 	}
 
 	if cmd.add {
@@ -743,10 +747,10 @@ func getProjectConstraint(arg string, sm gps.SourceManager) (gps.ProjectConstrai
 	return gps.ProjectConstraint{Ident: pi, Constraint: c}, arg, nil
 }
 
-func checkErrors(m map[string]pkgtree.PackageOrErr) error {
+func checkErrors(m map[string]pkgtree.PackageOrErr) (fatal bool, err error) {
 	var (
-		buildErrors []string
-		noGoErrors  int
+		noGoErrors    int
+		pkgtreeErrors = make(pkgtreeErrs, 0, len(m))
 	)
 
 	for _, poe := range m {
@@ -755,18 +759,42 @@ func checkErrors(m map[string]pkgtree.PackageOrErr) error {
 			case *build.NoGoError:
 				noGoErrors++
 			default:
-				buildErrors = append(buildErrors, poe.Err.Error())
+				pkgtreeErrors = append(pkgtreeErrors, poe.Err)
 			}
 		}
 	}
 
+	// If pkgtree was empty or all dirs lacked any Go code, return an error.
 	if len(m) == 0 || len(m) == noGoErrors {
-		return errors.New("all dirs lacked any go code")
+		return true, errors.New("no dirs contained any Go code")
 	}
 
-	if len(buildErrors) > 0 {
-		return errors.Errorf("Found %d errors:\n\n%s", len(buildErrors), strings.Join(buildErrors, "\n"))
+	// If all dirs contained build errors, return an error.
+	if len(m) == len(pkgtreeErrors) {
+		return true, errors.New("all dirs contained build errors")
 	}
 
-	return nil
+	// If all directories either had no Go files or caused a build error, return an error.
+	if len(m) == len(pkgtreeErrors)+noGoErrors {
+		return true, pkgtreeErrors
+	}
+
+	// If m contained some errors, return a warning with those errors.
+	if len(pkgtreeErrors) > 0 {
+		return false, pkgtreeErrors
+	}
+
+	return false, nil
+}
+
+type pkgtreeErrs []error
+
+func (e pkgtreeErrs) Error() string {
+	errs := make([]string, 0, len(e))
+
+	for _, err := range e {
+		errs = append(errs, err.Error())
+	}
+
+	return fmt.Sprintf("found %d errors in the package tree:\n%s", len(e), strings.Join(errs, "\n"))
 }

cmd/dep/ensure_test.go

@@ -56,21 +56,21 @@ func TestInvalidEnsureFlagCombinations(t *testing.T) {
 func TestCheckErrors(t *testing.T) {
 	tt := []struct {
 		name        string
-		hasErrs     bool
+		fatal       bool
 		pkgOrErrMap map[string]pkgtree.PackageOrErr
 	}{
 		{
-			name:    "noErrors",
-			hasErrs: false,
+			name:  "noErrors",
+			fatal: false,
 			pkgOrErrMap: map[string]pkgtree.PackageOrErr{
 				"mypkg": {
 					P: pkgtree.Package{},
 				},
 			},
 		},
 		{
-			name:    "hasErrors",
-			hasErrs: true,
+			name:  "hasErrors",
+			fatal: true,
 			pkgOrErrMap: map[string]pkgtree.PackageOrErr{
 				"github.com/me/pkg": {
 					Err: &build.NoGoError{},
@@ -81,8 +81,8 @@ func TestCheckErrors(t *testing.T) {
 			},
 		},
 		{
-			name:    "onlyGoErrors",
-			hasErrs: false,
+			name:  "onlyGoErrors",
+			fatal: false,
 			pkgOrErrMap: map[string]pkgtree.PackageOrErr{
 				"github.com/me/pkg": {
 					Err: &build.NoGoError{},
@@ -93,20 +93,48 @@ func TestCheckErrors(t *testing.T) {
 			},
 		},
 		{
-			name:    "allGoErrors",
-			hasErrs: true,
+			name:  "onlyBuildErrors",
+			fatal: false,
 			pkgOrErrMap: map[string]pkgtree.PackageOrErr{
 				"github.com/me/pkg": {
 					Err: &build.NoGoError{},
 				},
+				"github.com/someone/pkg": {
+					P: pkgtree.Package{},
+				},
+			},
+		},
+		{
+			name:  "allGoErrors",
+			fatal: true,
+			pkgOrErrMap: map[string]pkgtree.PackageOrErr{
+				"github.com/me/pkg": {
+					Err: &build.NoGoError{},
+				},
+			},
+		},
+		{
+			name:  "allMixedErrors",
+			fatal: true,
+			pkgOrErrMap: map[string]pkgtree.PackageOrErr{
+				"github.com/me/pkg": {
+					Err: &build.NoGoError{},
+				},
+				"github.com/someone/pkg": {
+					Err: errors.New("code is busted"),
+				},
 			},
 		},
 	}
 
 	for _, tc := range tt {
 		t.Run(tc.name, func(t *testing.T) {
-			if hasErrs := checkErrors(tc.pkgOrErrMap) != nil; hasErrs != tc.hasErrs {
-				t.Fail()
+			fatal, err := checkErrors(tc.pkgOrErrMap)
+			if tc.fatal != fatal {
+				t.Fatalf("expected fatal flag to be %T, got %T", tc.fatal, fatal)
+			}
+			if err == nil && fatal {
+				t.Fatal("unexpected fatal flag value while err is nil")
 			}
 		})
 	}

cmd/dep/testdata/harness_tests/ensure/pkg-errors/case1/testcase.json

@@ -3,6 +3,6 @@
     ["init", "-no-examples", "-skip-tools"],
     ["ensure", "-update"]
   ],
-  "error-expected": "all dirs lacked any go code",
+  "error-expected": "no dirs contained any Go code",
   "vendor-final": []
 }

cmd/dep/testdata/harness_tests/ensure/pkg-errors/case2/testcase.json

@@ -2,6 +2,6 @@
     "commands": [
         ["ensure"]
     ],
-    "error-expected": "Found 1 errors",
+    "error-expected": "found 1 errors",
     "vendor-final": []
-}
+}