Gerrit Bot (Gerrit)

unread,

11:09 AM (6 hours ago) 11:09 AM

to goph...@pubsubhelper.golang.org, golang-co...@googlegroups.com

Gerrit Bot has uploaded the change for review

Commit message

fix: fix issue 78183 of cmd/go: report importer-specific positions in go list dependency errors

This PR will be imported into Gerrit with the title and first
comment (this text) used to generate the subject and body of
the Gerrit change.

cmd/go: report importer-specific positions in go list dependency errors

When multiple packages import the same missing dependency, go list may
reuse a shared PackageError and report the same Pos for every importer.
This causes incorrect locations in DepsErrors and confuses downstream
tools that rely on precise positions.

This change records whether a PackageError position refers to the
importer side, then in collectDepsErrors clones dependency errors per
importer. For importer-side errors, it rewrites Pos using the current
package's ImportPos for that dependency (with short path formatting),
so each importing package gets its own correct location.

Fixes #78183.

Change-Id: I98680f2950663661d00fbc1b157ddadde427282b

GitHub-Last-Rev: d313611e99c62d4e5420dd9dbd804757ec025f66

GitHub-Pull-Request: golang/go#78206

Change diff

diff --git a/src/cmd/go/internal/list/list.go b/src/cmd/go/internal/list/list.go
index 6df9c33..b17c084 100644
--- a/src/cmd/go/internal/list/list.go
+++ b/src/cmd/go/internal/list/list.go
@@ -936,11 +936,23 @@
 func collectDepsErrors(p *load.Package) {
 	depsErrors := make(map[*load.PackageError]bool)
 
-	for _, p := range p.Internal.Imports {
-		if p.Error != nil {
-			depsErrors[p.Error] = true
+	for _, dep := range p.Internal.Imports {
+		if dep.Error != nil {
+			// The cached Package for dep may be shared by multiple importers.
+			// Clone the error and, if Pos refers to the importer side,
+			// replace it with p's own import position (#78183).
+			depErr := *dep.Error
+			if depErr.IsImporterPos() && p.Internal.Build != nil {
+				if pos := p.Internal.Build.ImportPos[dep.ImportPath]; len(pos) > 0 {
+					p0 := pos[0]
+					p0.Filename = base.ShortPath(p0.Filename)
+					depErr.Pos = p0.String()
+				}
+			}
+			depsErrors[&depErr] = true
 		}
-		for _, q := range p.DepsErrors {
+
+		for _, q := range dep.DepsErrors {
 			depsErrors[q] = true
 		}
 	}
diff --git a/src/cmd/go/internal/load/pkg.go b/src/cmd/go/internal/load/pkg.go
index e2a77d7..1d1aa9d 100644
--- a/src/cmd/go/internal/load/pkg.go
+++ b/src/cmd/go/internal/load/pkg.go
@@ -457,6 +457,7 @@
 	Err              error       // the error itself
 	IsImportCycle    bool        // the error is an import cycle
 	alwaysPrintStack bool        // whether to always print the ImportStack
+	importerPos      bool        // Pos refers to the importer, not the imported package
 }
 
 func (p *PackageError) Error() string {
@@ -509,6 +510,13 @@
 	pos := posList[0]
 	pos.Filename = base.ShortPath(pos.Filename)
 	p.Pos = pos.String()
+	p.importerPos = true
+}
+
+// IsImporterPos reports whether Pos refers to the import statement
+// in the importing package, not to the imported package's source.
+func (p *PackageError) IsImporterPos() bool {
+	return p.importerPos
 }
 
 // ImportPathError is a type of error that prevents a package from being loaded
diff --git a/src/cmd/go/testdata/script/golist_missingimport.txt b/src/cmd/go/testdata/script/golist_missingimport.txt
new file mode 100644
index 0000000..436ad0f
--- /dev/null
+++ b/src/cmd/go/testdata/script/golist_missingimport.txt
@@ -0,0 +1,26 @@
+# Test that go list reports correct Pos for each import of a missing package.
+# When two different packages import the same missing package, each should
+# report the position of its own import statement, not reuse the first one.
+# See issue #78183.
+
+env GOWORK=off
+
+go list -e -f '{{range .DepsErrors}}package {{$.ImportPath}}: error at {{.Pos}}: {{.Err}}{{end}}' ./...
+
+stdout 'package example.com/test/p: error at p/p.go:3:8: no required module'
+stdout 'package example.com/test/q: error at q/q.go:3:8: no required module'
+
+-- go.mod --
+module example.com/test
+
+go 1.22
+
+-- p/p.go --
+package p
+
+import _ "example.com/missing"
+
+-- q/q.go --
+package q
+
+import _ "example.com/missing"
\ No newline at end of file

Change information

Files:

M src/cmd/go/internal/list/list.go
M src/cmd/go/internal/load/pkg.go
A src/cmd/go/testdata/script/golist_missingimport.txt

Change size: M

Delta: 3 files changed, 50 insertions(+), 4 deletions(-)

Open in Gerrit

Related details

Attention set is empty

Submit Requirements:

Code-Review
No-Unresolved-Comments
Review-Enforcement
TryBots-Pass

Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings.

Gerrit

unsatisfied_requirement

satisfied_requirement

open

diffy

Gopher Robot (Gerrit)

unread,

11:13 AM (6 hours ago) 11:13 AM

to Gerrit Bot, goph...@pubsubhelper.golang.org, golang-co...@googlegroups.com

Message from Gopher Robot

Congratulations on opening your first change. Thank you for your contribution!

Next steps:
A maintainer will review your change and provide feedback. See
https://go.dev/doc/contribute#review for more info and tips to get your
patch through code review.

Most changes in the Go project go through a few rounds of revision. This can be
surprising to people new to the project. The careful, iterative review process
is our way of helping mentor contributors and ensuring that their contributions
have a lasting impact.

During May-July and Nov-Jan the Go project is in a code freeze, during which
little code gets reviewed or merged. If a reviewer responds with a comment like
R=go1.11 or adds a tag like "wait-release", it means that this CL will be
reviewed as part of the next development cycle. See https://go.dev/s/release
for more details.

Open in Gerrit

Related details

Attention set is empty

Submit Requirements:

Code-Review
No-Unresolved-Comments
Review-Enforcement
TryBots-Pass

Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings.

Gerrit

unsatisfied_requirement

satisfied_requirement

open

diffy

Alan Donovan (Gerrit)

unread,

4:04 PM (1 hour ago) 4:04 PM

to Gerrit Bot, goph...@pubsubhelper.golang.org, Michael Matloob, Gopher Robot, golang-co...@googlegroups.com

Attention needed from Michael Matloob

Alan Donovan added 3 comments

Patchset-level comments

File-level comment, Patchset 1 (Latest):

Alan Donovan . resolved

Thanks for contributing this fix!

Commit Message

Line 6, Patchset 1 (Latest):


fix: fix issue 78183 of cmd/go: report importer-specific positions in go list dependency errors

This PR will be imported into Gerrit with the title and first
comment (this text) used to generate the subject and body of
the Gerrit change.

Alan Donovan . unresolved

delete

File src/cmd/go/internal/list/list.go

Line 944, Patchset 1 (Latest): depErr := *dep.Error

Alan Donovan . unresolved

new(dep.Error)

Open in Gerrit

Related details

Attention is currently required from:

Michael Matloob

Submit Requirements:

Code-Review

No-Unresolved-Comments

Review-Enforcement
TryBots-Pass

Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings.

Gerrit

unsatisfied_requirement

open

diffy

Reply all

Reply to author

Forward

[go] fix: fix issue 78183 of cmd/go: report importer-specific positions in go list dependency errors

Gerrit Bot (Gerrit)

Gerrit Bot has uploaded the change for review

Commit message

Change diff

Change information

Related details

Gopher Robot (Gerrit)

Message from Gopher Robot

Related details

Alan Donovan (Gerrit)

Alan Donovan added 3 comments

Related details