cmd/go: diagnose import cycles better

Before this CL, the import stack was a) not printed and b) overwritten later
in the build, destroying the information about the cycle. This CL fixes both.

I made time depend on os (os already depends on time) and with this CL the error is:

/Users/r/go/src/pkg/fmt/print.go:10:2: import cycle not allowed
package code.google.com/p/XXX/YYY:
        imports fmt
        imports os
        imports time
        imports os

Doesn't give line numbers for the actual imports, as requested in the bug, but
I don't believe that's important.

Fixes #4292.

R=golang-dev, adg
CC=golang-dev
https://golang.org/cl/13100043

src/cmd/go/pkg.go

@@ -129,12 +129,17 @@ func (p *Package) copyBuild(pp *build.Package) {
 
 // A PackageError describes an error loading information about a package.
 type PackageError struct {
-	ImportStack []string // shortest path from package named on command line to this one
-	Pos         string   // position of error
-	Err         string   // the error itself
+	ImportStack   []string // shortest path from package named on command line to this one
+	Pos           string   // position of error
+	Err           string   // the error itself
+	isImportCycle bool     // the error is an import cycle
 }
 
 func (p *PackageError) Error() string {
+	// Import cycles deserve special treatment.
+	if p.isImportCycle {
+		return fmt.Sprintf("%s: %s\npackage %s\n", p.Pos, p.Err, strings.Join(p.ImportStack, "\n\timports "))
+	}
 	if p.Pos != "" {
 		// Omit import stack.  The full path to the file where the error
 		// is the most important thing.
@@ -271,13 +276,16 @@ func reusePackage(p *Package, stk *importStack) *Package {
 	if p.imports == nil {
 		if p.Error == nil {
 			p.Error = &PackageError{
-				ImportStack: stk.copy(),
-				Err:         "import cycle not allowed",
+				ImportStack:   stk.copy(),
+				Err:           "import cycle not allowed",
+				isImportCycle: true,
 			}
 		}
 		p.Incomplete = true
 	}
-	if p.Error != nil && stk.shorterThan(p.Error.ImportStack) {
+	// Don't rewrite the import stack in the error if we have an import cycle.
+	// If we do, we'll lose the path that describes the cycle.
+	if p.Error != nil && !p.Error.isImportCycle && stk.shorterThan(p.Error.ImportStack) {
 		p.Error.ImportStack = stk.copy()
 	}
 	return p