x/tools/go/packages: "conflicting information" error when importing internal or main package

[jayconrod]

Suppose we have a module with two packages, a and b where a imports b. If b is a main package or is in an internal directory not visible to a, go list -e -deps -json a b will report b twice: once as a dummy package containing an error for a's bad import, and again as the real b package.

As a concrete example:

-- a/a.go --
package a

import _ "example.com/m/b"
-- b/b.go --
package main
-- go.mod --
module example.com/m

go 1.13

The go list command prints example.com/m/b twice. This is working as intended (though perhaps not as expected).

$ go list -e -deps ./...
internal/cpu
unsafe
internal/bytealg
runtime/internal/atomic
runtime/internal/sys
runtime/internal/math
runtime
example.com/m/b
example.com/m/a
example.com/m/b

golang.org/x/tools/go/packages gets confused by this.

$ gopackages ./a ./b
gopackages: internal error: go list gives conflicting information for package example.com/m/b

Internally, go/packages runs go list. If it sees a package more than once with different data, it reports this error.

These kinds of errors should have positions with file paths in the importing package. Perhaps go/packages could figure out which package contains the file with the error, then attach the error to that package.

cc @matloob

[matloob]

It looks like the importing package already includes the error, at least in this case (see snippet of go list -json output below). I'm wondering if we can just drop the second version of the package, with the error. Are there cases where the error doesn't appear on the importing package?

I'm also wondering if why we need the second version of the package since the error already appears on the importing package.

{
	"Dir": "/Users/matloob/Desktop/m/b",
	"ImportPath": "example.com/m/b",
	"Name": "main",
	"Target": "/Users/matloob/go/bin/b",
	"Root": "/Users/matloob/Desktop/m",
	"Module": {
		"Path": "example.com/m",
		"Main": true,
		"Dir": "/Users/matloob/Desktop/m",
		"GoMod": "/Users/matloob/Desktop/m/go.mod",
		"GoVersion": "1.13"
	},
	"DepOnly": true,
	"Stale": true,
	"StaleReason": "build ID mismatch",
	"GoFiles": [
		"b.go"
	],
	"Deps": [
		"internal/bytealg",
		"internal/cpu",
		"runtime",
		"runtime/internal/atomic",
		"runtime/internal/math",
		"runtime/internal/sys",
		"unsafe"
	],
	"Error": {
		"ImportStack": [
			"example.com/m/a",
			"example.com/m/b"
		],
		"Pos": "a/a.go:3:8",
		"Err": "import \"example.com/m/b\" is a program, not an importable package"
	}
}
{
	"Dir": "/Users/matloob/Desktop/m/a",
	"ImportPath": "example.com/m/a",
	"Name": "a",
	"Root": "/Users/matloob/Desktop/m",
	"Module": {
		"Path": "example.com/m",
		"Main": true,
		"Dir": "/Users/matloob/Desktop/m",
		"GoMod": "/Users/matloob/Desktop/m/go.mod",
		"GoVersion": "1.13"
	},
	"Match": [
		"./..."
	],
	"Stale": true,
	"StaleReason": "stale dependency: example.com/m/b",
	"GoFiles": [
		"a.go"
	],
	"Imports": [
		"example.com/m/b"
	],
	"Deps": [
		"example.com/m/b",
		"internal/bytealg",
		"internal/cpu",
		"runtime",
		"runtime/internal/atomic",
		"runtime/internal/math",
		"runtime/internal/sys",
		"unsafe"
	],
	"DepsErrors": [
		{
			"ImportStack": [
				"example.com/m/a",
				"example.com/m/b"
			],
			"Pos": "a/a.go:3:8",
			"Err": "import \"example.com/m/b\" is a program, not an importable package"
		}
	]
}
{
	"Dir": "/Users/matloob/Desktop/m/b",
	"ImportPath": "example.com/m/b",
	"Name": "main",
	"Target": "/Users/matloob/go/bin/b",
	"Root": "/Users/matloob/Desktop/m",
	"Module": {
		"Path": "example.com/m",
		"Main": true,
		"Dir": "/Users/matloob/Desktop/m",
		"GoMod": "/Users/matloob/Desktop/m/go.mod",
		"GoVersion": "1.13"
	},
	"Match": [
		"./..."
	],
	"Stale": true,
	"StaleReason": "build ID mismatch",
	"GoFiles": [
		"b.go"
	],
	"Deps": [
		"internal/bytealg",
		"internal/cpu",
		"runtime",
		"runtime/internal/atomic",
		"runtime/internal/math",
		"runtime/internal/sys",
		"unsafe"
	]
}

[jayconrod]

Do you mean the DepsErrors field in example.com/m/a? DepsErrors is a list of errors that were found in dependencies. It's constructed by copying the Error field from all the packages in Deps (which is the transitive closure of Imports). So the error is reported in two places, but it's really only attached to one package.

I think the right behavior of x/tools/go/packages is to find errors in incomplete packages with a position in an importing package. Those errors should be reported through the importing packages, and the error-only packages should not be included in the output.

Ideally go list -e -json would work that way, too, but it would require an incompatible change to the output format.

[matloob]

Okay, I see. I neglected to see that the field on the 'real' version of the package was DepsErrors. But I'm still curious then if it's possible to bubble up the DepsErrors (since an error in the deps should mean an error in the package itself) and drop the version of the package with the error itself.

Is that what you're proposing for the right behavior of go/packages? That we find DepsErrors on broken packages and bubble them up? Also, by incomplete, do we mean that the type information is incomplete? Do we have that information if we haven't built the package?

[jayconrod]

But I'm still curious then if it's possible to bubble up the DepsErrors (since an error in the deps should mean an error in the package itself) and drop the version of the package with the error itself.

I don't think it's possible in general. The go list -e -json format only allows one error per package. What happens if a package imports two different main packages? We'd either have to change the format to allow multiple errors or ignore everything after the first error.

Is that what you're proposing for the right behavior of go/packages? That we find DepsErrors on broken packages and bubble them up?

Yes exactly.

Also, by incomplete, do we mean that the type information is incomplete? Do we have that information if we haven't built the package?

I was thinking of the Incomplete field, but that's not actually set in this situation.

Maybe a simplification: if we see a package with Error set with a position in the importing package's source file (according to .Error.Pos and .Error.ImportStack), gopackages should report the error as part of the importing package and should otherwise ignore that package from go list (it should not conflict with other packages with the same ImportPath).

[gopherbot]

Change https://golang.org/cl/216721 mentions this issue: go/packages: add a workaround for golang/go#36188