Skip to content
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

Jump to bottom

x/tools/go/packages: cgo error in one package causes entire Load to fail #31462

Closed
dominikh opened this issue Apr 14, 2019 · 4 comments
Closed

x/tools/go/packages: cgo error in one package causes entire Load to fail #31462

dominikh opened this issue Apr 14, 2019 · 4 comments
Labels
FrozenDueToAge NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. Tools This label describes issues relating to any tools in the x/tools repository.
Milestone
Unreleased

Comments

@dominikh
Copy link
Member

What version of Go are you using (go version)?

$ go version
go version go1.12.3 linux/amd64
go version devel +a01d108e30 Sun Apr 14 16:19:31 2019 +0000 linux/amd64

Trying to load multiple packages with packages.Load fails if any of the packages have cgo errors:

desktop 1? ~ $ cat $GOPATH/src/sandbox/foo/*.go
package main

import (
	"fmt"
)

func main() {
	x := 42
	fmt.Printf("%w\n", x)
}
desktop ~ $ cat $GOPATH/src/sandbox/bar/*.go
package bar

import "C"

func fn() {
	C.doesnotexist()
}
desktop ~ $ cat /tmp/foo.go 
package main

import (
	"log"

	"golang.org/x/tools/go/packages"
)

func main() {
	cfg := &packages.Config{
		Mode: packages.LoadAllSyntax,
	}
	_, err := packages.Load(cfg, "sandbox/foo", "sandbox/bar")
	if err != nil {
		log.Fatal(err)
	}
}
desktop ~ $ go run /tmp/foo.go 
2019/04/15 01:13:57 go [list -e -json -compiled=true -test=false -export=false -deps=true -find=false -- sandbox/foo sandbox/bar]: exit status 2: # sandbox/bar
prj/src/sandbox/bar/bar.go:6:2: could not determine kind of name for C.doesnotexist
exit status 1
desktop 1? ~ $

I expected packages.Load to return no error, to get two packages for sandbox/foo and sandbox/bar, and sandbox/bar to have a non-empty Errors.

/cc @ianthehat @matloob
@gopherbot gopherbot added this to the Unreleased milestone Apr 14, 2019
@dmitshur dmitshur added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Apr 29, 2019
@sauyon
Copy link

sauyon commented Jun 28, 2019 •
edited

After discussion with @dominikh on IRC, we've worked out the source of the bug:

cgo preprocessing always happens, and the tool doesn't expect there to be build output/failures unless the function usesExportData(cfg) evaluates to true. This causes Load function to think the driver has failed, and therefore error out before doing any analysis. Interestingly, the error code seems to only get set if -compiled=true is passed.

This bug will therefore only trigger if packages.NeedExportsFile is not set and either packages.NeedTypes is not set or packages.NeedTypesInfo is set.

Relevant code:
usesExportData: https://github.com/golang/tools/blob/212fb13d595e5faf79425c78ae101012873a81a1/go/packages/packages.go#L1073
Relevant exception: https://github.com/golang/tools/blob/212fb13d595e5faf79425c78ae101012873a81a1/go/packages/golist.go#L818

EDIT: Easy workaround for now it set packages.NeedExportsFile

sauyon added a commit to sauyon/tools that referenced this issue Jul 4, 2019
@sauyon
Ignore more errors in go list
be4f779 
Adds some hacks to work around go list bugs causing golang/go#31462 and
golang/go#32755.

In addition, appends error messages generated in this manner to stdout instead
of discarding stdout entirely, and ensures that this does not cause a
conflicting package error by adding an internal field to packageJson.
@sauyon sauyon mentioned this issue Jul 4, 2019
Closed
@gopherbot
Copy link

Change https://golang.org/cl/185077 mentions this issue: Ignore more errors in go list

sauyon added a commit to sauyon/tools that referenced this issue Jul 4, 2019
@sauyon
go/packages: Ignore more errors in go list
24b94ac 
Adds some hacks to work around go list bugs causing golang/go#31462 and
golang/go#32755.

In addition, appends error messages generated in this manner to stdout instead
of discarding stdout entirely, and ensures that this does not cause a
conflicting package error by adding an internal field to packageJson.
sauyon added a commit to sauyon/tools that referenced this issue Jul 4, 2019
@sauyon
go/packages: Ignore more errors in go list
68654d0 
Adds some hacks to work around go list bugs causing golang/go#31462 and
golang/go#32755.

In addition, appends error messages generated in this manner to stdout instead
of discarding stdout entirely, and ensures that this does not cause a
conflicting package error by adding an internal field to packageJson.
sauyon added a commit to sauyon/tools that referenced this issue Jul 4, 2019
@sauyon
go/packages: Ignore more errors in go list
a0370a2 
Adds some hacks to work around go list bugs causing golang/go#31462 and
golang/go#32755.

In addition, appends error messages generated in this manner to stdout instead
of discarding stdout entirely, and ensures that this does not cause a
conflicting package error by adding an internal field to packageJson.
sauyon added a commit to sauyon/tools that referenced this issue Jul 4, 2019
@sauyon
go/packages: Ignore more errors in go list
0a80f6c 
Adds some hacks to work around go list bugs causing golang/go#31462 and
golang/go#32755.

In addition, appends error messages generated in this manner to stdout instead
of discarding stdout entirely, and ensures that this does not cause a
conflicting package error by adding an internal field to packageJson.
sauyon added a commit to sauyon/tools that referenced this issue Sep 8, 2019
@sauyon
go/packages: Ignore more errors in go list
fdded40 
Adds some hacks to work around go list bugs causing golang/go#31462 and
golang/go#32755.

In addition, appends error messages generated in this manner to stdout instead
of discarding stdout entirely, and ensures that this does not cause a
conflicting package error by adding an internal field to packageJson.
@gopherbot gopherbot added the Tools This label describes issues relating to any tools in the x/tools repository. label Sep 12, 2019
sauyon added a commit to sauyon/tools that referenced this issue Sep 12, 2019
@sauyon
go/packages: Ignore more errors in go list
601be70 
Adds some hacks to work around go list bugs causing golang/go#31462 and
golang/go#32755.

In addition, appends error messages generated in this manner to stdout instead
of discarding stdout entirely, and ensures that this does not cause a
conflicting package error by adding an internal field to packageJson.
@matloob
Copy link
Contributor

matloob commented Oct 29, 2019

This is fixed by golang.org/cl/192330.

@matloob matloob closed this as completed Oct 29, 2019
@matloob
Copy link
Contributor

matloob commented Oct 29, 2019

This is fixed by golang.org/cl/192330

@golang golang locked and limited conversation to collaborators Oct 28, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
FrozenDueToAge NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. Tools This label describes issues relating to any tools in the x/tools repository.
Projects
None yet
Milestone
Unreleased
Development

No branches or pull requests
5 participants
@dominikh @dmitshur @sauyon @gopherbot @matloob