6 files changed, 90 insertions(+), 38 deletions(-)

M src/cmd/go/internal/list/list.go
M src/cmd/go/internal/load/pkg.go
M src/cmd/go/internal/load/test.go
M src/cmd/go/internal/work/action.go
M src/cmd/go/testdata/script/list_test_cycle.txt
M src/cmd/go/testdata/script/mod_import_cycle.txt

@@ 967,7 967,7 @@ func collectDepsErrors(p *load.Package) {
 			return false
 		}
 		pathi, pathj := stki[len(stki)-1], stkj[len(stkj)-1]
-		return pathi < pathj
+		return pathi.Pkg < pathj.Pkg
 	})
 }
 

@@ 327,7 327,7 @@ func (p *Package) setLoadPackageDataError(err error, path string, stk *ImportSta
 	// move the modload errors into this package to avoid a package import cycle,
 	// and from having to export an error type for the errors produced in build.
 	if !isMatchErr && (nogoErr != nil || isScanErr) {
-		stk.Push(path)
+		stk.Push(ImportInfo{Pkg: path, Pos: extractFirstImport(importPos)})
 		defer stk.Pop()
 	}
 


@@ 338,7 338,8 @@ func (p *Package) setLoadPackageDataError(err error, path string, stk *ImportSta
 	}
 	p.Incomplete = true
 
-	if path != stk.Top() {
+	top, ok := stk.Top()
+	if ok && path != top.Pkg {
 		p.Error.setPos(importPos)
 	}
 }


@@ 455,11 456,11 @@ func (p *Package) copyBuild(opts PackageOpts, 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              error    // the error itself
-	IsImportCycle    bool     // the error is an import cycle
-	alwaysPrintStack bool     // whether to always print the ImportStack
+	ImportStack      ImportStack // shortest path from package named on command line to this one with position
+	Pos              string      // position of error
+	Err              error       // the error itself
+	IsImportCycle    bool        // the error is an import cycle
+	alwaysPrintStack bool        // whether to always print the ImportStack
 }
 
 func (p *PackageError) Error() string {


@@ 485,7 486,11 @@ func (p *PackageError) Error() string {
 	if p.Pos != "" {
 		optpos = "\n\t" + p.Pos
 	}
-	return "package " + strings.Join(p.ImportStack, "\n\timports ") + optpos + ": " + p.Err.Error()
+	imports := p.ImportStack.Pkgs()
+	if p.IsImportCycle {
+		imports = p.ImportStack.PkgsWithPos()
+	}
+	return "package " + strings.Join(imports, "\n\timports ") + optpos + ": " + p.Err.Error()
 }
 
 func (p *PackageError) Unwrap() error { return p.Err }


@@ 494,10 499,10 @@ func (p *PackageError) Unwrap() error { return p.Err }
 // and non-essential fields are omitted.
 func (p *PackageError) MarshalJSON() ([]byte, error) {
 	perr := struct {
-		ImportStack []string
+		ImportStack []string // use []string for package names
 		Pos         string
 		Err         string
-	}{p.ImportStack, p.Pos, p.Err.Error()}
+	}{p.ImportStack.Pkgs(), p.Pos, p.Err.Error()}
 	return json.Marshal(perr)
 }
 


@@ 558,12 563,21 @@ func (e *importError) ImportPath() string {
 	return e.importPath
 }
 
+type ImportInfo struct {
+	Pkg string
+	Pos *token.Position
+}
+
 // An ImportStack is a stack of import paths, possibly with the suffix " (test)" appended.
 // The import path of a test package is the import path of the corresponding
 // non-test package with the suffix "_test" added.
-type ImportStack []string
+type ImportStack []ImportInfo
+
+func NewImportInfo(pkg string, pos *token.Position) ImportInfo {
+	return ImportInfo{Pkg: pkg, Pos: pos}
+}
 
-func (s *ImportStack) Push(p string) {
+func (s *ImportStack) Push(p ImportInfo) {
 	*s = append(*s, p)
 }
 


@@ 571,15 585,35 @@ func (s *ImportStack) Pop() {
 	*s = (*s)[0 : len(*s)-1]
 }
 
-func (s *ImportStack) Copy() []string {
-	return append([]string{}, *s...)
+func (s *ImportStack) Copy() ImportStack {
+	return slices.Clone(*s)
+}
+
+func (s *ImportStack) Pkgs() []string {
+	ss := make([]string, 0, len(*s))
+	for _, v := range *s {
+		ss = append(ss, v.Pkg)
+	}
+	return ss
+}
+
+func (s *ImportStack) PkgsWithPos() []string {
+	ss := make([]string, 0, len(*s))
+	for _, v := range *s {
+		if v.Pos != nil {
+			ss = append(ss, v.Pkg+" from "+filepath.Base(v.Pos.Filename))
+		} else {
+			ss = append(ss, v.Pkg)
+		}
+	}
+	return ss
 }
 
-func (s *ImportStack) Top() string {
+func (s *ImportStack) Top() (ImportInfo, bool) {
 	if len(*s) == 0 {
-		return ""
+		return ImportInfo{}, false
 	}
-	return (*s)[len(*s)-1]
+	return (*s)[len(*s)-1], true
 }
 
 // shorterThan reports whether sp is shorter than t.


@@ 592,8 626,9 @@ func (sp *ImportStack) shorterThan(t []string) bool {
 	}
 	// If they are the same length, settle ties using string ordering.
 	for i := range s {
-		if s[i] != t[i] {
-			return s[i] < t[i]
+		siPkg := s[i].Pkg
+		if siPkg != t[i] {
+			return siPkg < t[i]
 		}
 	}
 	return false // they are equal


@@ 655,7 690,7 @@ const (
 	cmdlinePkgLiteral
 )
 
-// LoadPackage does Load import, but without a parent package load contezt
+// LoadPackage does Load import, but without a parent package load context
 func LoadPackage(ctx context.Context, opts PackageOpts, path, srcDir string, stk *ImportStack, importPos []token.Position, mode int) *Package {
 	p, err := loadImport(ctx, opts, nil, path, srcDir, nil, stk, importPos, mode)
 	if err != nil {


@@ 707,7 742,7 @@ func loadImport(ctx context.Context, opts PackageOpts, pre *preload, path, srcDi
 			// sequence that empirically doesn't trigger for these errors, guarded by
 			// a somewhat complex condition. Figure out how to generalize that
 			// condition and eliminate the explicit calls here.
-			stk.Push(path)
+			stk.Push(ImportInfo{Pkg: path, Pos: extractFirstImport(importPos)})
 			defer stk.Pop()
 		}
 		p.setLoadPackageDataError(err, path, stk, nil)


@@ 726,7 761,7 @@ func loadImport(ctx context.Context, opts PackageOpts, pre *preload, path, srcDi
 	importPath := bp.ImportPath
 	p := packageCache[importPath]
 	if p != nil {
-		stk.Push(path)
+		stk.Push(ImportInfo{Pkg: path, Pos: extractFirstImport(importPos)})
 		p = reusePackage(p, stk)
 		stk.Pop()
 		setCmdline(p)


@@ 792,6 827,13 @@ func loadImport(ctx context.Context, opts PackageOpts, pre *preload, path, srcDi
 	return p, nil
 }
 
+func extractFirstImport(importPos []token.Position) *token.Position {
+	if len(importPos) == 0 {
+		return nil
+	}
+	return &importPos[0]
+}
+
 // loadPackageData loads information needed to construct a *Package. The result
 // is cached, and later calls to loadPackageData for the same package will return
 // the same data.


@@ 1412,7 1454,8 @@ func reusePackage(p *Package, stk *ImportStack) *Package {
 	}
 	// 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) {
+	if p.Error != nil && p.Error.ImportStack != nil &&
+		!p.Error.IsImportCycle && stk.shorterThan(p.Error.ImportStack.Pkgs()) {
 		p.Error.ImportStack = stk.Copy()
 	}
 	return p


@@ 1739,7 1782,8 @@ func (p *Package) load(ctx context.Context, opts PackageOpts, path string, stk *
 			// then the cause of the error is not within p itself: the error
 			// must be either in an explicit command-line argument,
 			// or on the importer side (indicated by a non-empty importPos).
-			if path != stk.Top() && len(importPos) > 0 {
+			top, ok := stk.Top()
+			if ok && path != top.Pkg && len(importPos) > 0 {
 				p.Error.setPos(importPos)
 			}
 		}


@@ 1905,7 1949,7 @@ func (p *Package) load(ctx context.Context, opts PackageOpts, path string, stk *
 	// Errors after this point are caused by this package, not the importing
 	// package. Pushing the path here prevents us from reporting the error
 	// with the position of the import declaration.
-	stk.Push(path)
+	stk.Push(ImportInfo{Pkg: path, Pos: extractFirstImport(importPos)})
 	defer stk.Pop()
 
 	pkgPath := p.ImportPath

@@ 114,7 114,7 @@ func TestPackagesAndErrors(ctx context.Context, done func(), opts PackageOpts, p
 	var stk ImportStack
 	var testEmbed, xtestEmbed map[string][]string
 	var incomplete bool
-	stk.Push(p.ImportPath + " (test)")
+	stk.Push(ImportInfo{Pkg: p.ImportPath + " (test)"})
 	rawTestImports := str.StringList(p.TestImports)
 	for i, path := range p.TestImports {
 		p1, err := loadImport(ctx, opts, pre, path, p.Dir, p, &stk, p.Internal.Build.TestImportPos[path], ResolveImport)


@@ 141,7 141,7 @@ func TestPackagesAndErrors(ctx context.Context, done func(), opts PackageOpts, p
 	}
 	stk.Pop()
 
-	stk.Push(p.ImportPath + "_test")
+	stk.Push(ImportInfo{Pkg: p.ImportPath + "_test"})
 	pxtestNeedsPtest := false
 	var pxtestIncomplete bool
 	rawXTestImports := str.StringList(p.XTestImports)


@@ 304,7 304,7 @@ func TestPackagesAndErrors(ctx context.Context, done func(), opts PackageOpts, p
 
 	// The generated main also imports testing, regexp, and os.
 	// Also the linker introduces implicit dependencies reported by LinkerDeps.
-	stk.Push("testmain")
+	stk.Push(ImportInfo{Pkg: "testmain"})
 	deps := TestMainDeps // cap==len, so safe for append
 	if cover != nil && cfg.Experiment.CoverageRedesign {
 		deps = append(deps, "internal/coverage/cfile")


@@ 544,9 544,16 @@ func recompileForTest(pmain, preal, ptest, pxtest *Package) *PackageError {
 			// The stack is supposed to be in the order x imports y imports z.
 			// We collect in the reverse order: z is imported by y is imported
 			// by x, and then we reverse it.
-			var stk []string
+			var stk ImportStack
 			for p != nil {
-				stk = append(stk, p.ImportPath)
+				importer, ok := importerOf[p]
+				if importer == nil && ok { // we set importerOf[p] == nil for the initial set of packages p that are imports of ptest
+					importer = ptest
+				}
+				stk = append(stk, ImportInfo{
+					Pkg: p.ImportPath,
+					Pos: extractFirstImport(importer.Internal.Build.ImportPos[p.ImportPath]),
+				})
 				p = importerOf[p]
 			}
 			// complete the cycle: we set importer[p] = nil to break the cycle


@@ 554,9 561,10 @@ func recompileForTest(pmain, preal, ptest, pxtest *Package) *PackageError {
 			// back here since we reached nil in the loop above to demonstrate
 			// the cycle as (for example) package p imports package q imports package r
 			// imports package p.
-			stk = append(stk, ptest.ImportPath)
+			stk = append(stk, ImportInfo{
+				Pkg: ptest.ImportPath,
+			})
 			slices.Reverse(stk)
-
 			return &PackageError{
 				ImportStack:   stk,
 				Err:           errors.New("import cycle not allowed in test"),

@@ 631,7 631,7 @@ func (b *Builder) vetAction(mode, depMode BuildMode, p *load.Package) *Action {
 
 		// vet expects to be able to import "fmt".
 		var stk load.ImportStack
-		stk.Push("vet")
+		stk.Push(load.NewImportInfo("vet", nil))
 		p1, err := load.LoadImportWithFlags("fmt", p.Dir, p, &stk, nil, 0)
 		if err != nil {
 			base.Fatalf("unexpected error loading fmt package from package %s: %v", p.ImportPath, err)

@@ 15,9 15,9 @@ cmp stderr wanterr.txt
 
 -- wanterr.txt --
 go: can't load test package: package example/p
-	imports example/q
-	imports example/r
-	imports example/p: import cycle not allowed in test
+	imports example/q from p_test.go
+	imports example/r from q.go
+	imports example/p from r.go: import cycle not allowed in test
 -- go.mod --
 module example
 go 1.20

@@ 2,7 2,7 @@ env GO111MODULE=on
 
 # 'go list all' should fail with a reasonable error message
 ! go list all
-stderr '^package m\n\timports m/a\n\timports m/b\n\timports m/a: import cycle not allowed'
+stderr '^package m\n\timports m/a from m.go\n\timports m/b from a.go\n\timports m/a from b.go: import cycle not allowed'
 
 # 'go list -e' should not print to stderr, but should mark all three
 # packages (m, m/a, and m/b) as Incomplete.