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

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

Open
dominikh opened this issue Apr 14, 2019 · 2 comments

Comments

@dominikh
Copy link
Member

commented Apr 14, 2019

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

@sauyon

This comment has been minimized.

Copy link

commented Jun 28, 2019

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
Ignore more errors in go list
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

This comment has been minimized.

Copy link

commented Jul 4, 2019

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
go/packages: Ignore more errors in go list
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
go/packages: Ignore more errors in go list
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
go/packages: Ignore more errors in go list
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
go/packages: Ignore more errors in go list
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
go/packages: Ignore more errors in go list
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 label Sep 12, 2019

sauyon added a commit to sauyon/tools that referenced this issue Sep 12, 2019
go/packages: Ignore more errors in go list
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.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants
You can’t perform that action at this time.