cmd/go: report missing vendor visibility error

The logic for saving the list of packages was not always
preferring to keep error messages around correctly.
The missed error led to an internal consistency failure later.

Fixes #17119.

Change-Id: I9723b5d2518c25e2cac5249e6a7b907be95b521c
Reviewed-on: https://go-review.googlesource.com/31812
Run-TryBot: Russ Cox <rsc@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Quentin Smith <quentin@golang.org>

src/cmd/go/go_test.go

@@ -2912,6 +2912,17 @@ func TestGoGetUpdateAllDoesNotTryToLoadDuplicates(t *testing.T) {
 	tg.grepStderrNot("duplicate loads of", "did not remove old packages from cache")
 }
 
+// Issue 17119 more duplicate load errors
+func TestIssue17119(t *testing.T) {
+	testenv.MustHaveExternalNetwork(t)
+
+	tg := testgo(t)
+	defer tg.cleanup()
+	tg.setenv("GOPATH", filepath.Join(tg.pwd(), "testdata"))
+	tg.runFail("build", "dupload")
+	tg.grepBothNot("duplicate load|internal error", "internal error")
+}
+
 func TestFatalInBenchmarkCauseNonZeroExitStatus(t *testing.T) {
 	tg := testgo(t)
 	defer tg.cleanup()

src/cmd/go/pkg.go

@@ -970,6 +970,15 @@ func (p *Package) load(stk *importStack, bp *build.Package, err error) *Package
 	// Build list of imported packages and full dependency list.
 	imports := make([]*Package, 0, len(p.Imports))
 	deps := make(map[string]*Package)
+	save := func(path string, p1 *Package) {
+		// The same import path could produce an error or not,
+		// depending on what tries to import it.
+		// Prefer to record entries with errors, so we can report them.
+		if deps[path] == nil || p1.Error != nil {
+			deps[path] = p1
+		}
+	}
+
 	for i, path := range importPaths {
 		if path == "C" {
 			continue
@@ -1013,15 +1022,11 @@ func (p *Package) load(stk *importStack, bp *build.Package, err error) *Package
 		if i < len(p.Imports) {
 			p.Imports[i] = path
 		}
-		deps[path] = p1
+
+		save(path, p1)
 		imports = append(imports, p1)
 		for _, dep := range p1.deps {
-			// The same import path could produce an error or not,
-			// depending on what tries to import it.
-			// Prefer to record entries with errors, so we can report them.
-			if deps[dep.ImportPath] == nil || dep.Error != nil {
-				deps[dep.ImportPath] = dep
-			}
+			save(dep.ImportPath, dep)
 		}
 		if p1.Incomplete {
 			p.Incomplete = true

src/cmd/go/testdata/src/dupload/dupload.go

+package main
+
+import (
+	_"dupload/p2"
+	_ "p"
+)
+
+func main() {}

src/cmd/go/testdata/src/dupload/p/p.go

+package p

src/cmd/go/testdata/src/dupload/p2/p2.go

+package p2
+import _ "dupload/vendor/p"

src/cmd/go/testdata/src/dupload/vendor/p/p.go

+package p