-
Notifications
You must be signed in to change notification settings - Fork 17.7k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
cmd/go: some errors not reported with go list
#59186
Comments
Change https://go.dev/cl/477838 mentions this issue: |
The vendor_import test lists packages that are known bad (e.g. bad.go, invalid.go). Pass -e to permit error. The mod_vendor_auto test includes a package that imports a main package, which should be an error. Pass -e to permit error. Updates #59186. Change-Id: I3b63025c3935f55feda1a95151d4c688d0394644 Reviewed-on: https://go-review.googlesource.com/c/go/+/477838 Run-TryBot: Cherry Mui <cherryyz@google.com> TryBot-Result: Gopher Robot <gobot@golang.org> Reviewed-by: Bryan Mills <bcmills@google.com>
After CL https://golang.org/cl/474236, when listing multiple packages, the default -pgo=auto mode will cause this code https://cs.opensource.google/go/go/+/master:src/cmd/go/internal/list/list.go;l=736-788 to run, which will cause it to report the error. |
I thought blank imports of main packages were allowed as a way to track dependencies, eg when using the tools.go pattern. |
@seankhliao, |
I hit this through another vector, but similar. It seems very strange to assert that I can't list, when I am allowed to set up this construct in the first place. This is a breaking change - is it really worth it? |
@thockin, given that we don't understand why this ever worked differently in the first place, we can't easily restore that buggy behavior.
|
We hit this in kubernetes as well while trying to test with go1.21rc2: kubernetes/kubernetes#118996 |
@alexbozhenko, thanks for flagging that commit. That's an interesting clue, although it still doesn't really explain why the PGO changes seemed to trigger this behavior. 😅 But I think that will help us investigate. |
What version of Go are you using (
go version
)?tip (go1.21-4d9beb2052)
Does this issue reproduce with the latest release?
Yes
What operating system and processor architecture are you using (
go env
)?darwin/amd64
What did you do?
Having a module with two packages
a
andb
.b
is a main package.a
importsb
(erroneously).go.mod
a/a.go
b/b.go
Run
go list ./a ./b
.What did you expect to see?
Report the error for importing a non-importable package, like
What did you see instead?
No error, with 0 exit code.
Interestingly,
go list -test ./a ./b
reports the error, although the error has nothing to do with test files (there is none). The documentation doesn't seem to mention that the-test
flag can cause different behavior for error reporting.There are also some other errors not being reported with
go list
, but reported withgo list -test
. https://cs.opensource.google/go/go/+/master:src/cmd/go/testdata/script/vendor_import.txt is one example.bad.go
andinvalid.go
contains some errors, but thego list
command succeeded.It looks like the different behavior is related to that this code https://cs.opensource.google/go/go/+/master:src/cmd/go/internal/list/list.go;l=736-788 runs when
-test
is specified.Found while working on CL https://golang.org/cl/474236.
cc @bcmills
The text was updated successfully, but these errors were encountered: