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: all "no Go files in" errors get assigned to single package as a single error #32755

Closed
dominikh opened this issue Jun 24, 2019 · 5 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

Comments

@dominikh
Copy link
Member

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

$ go version
go version go1.12.5 linux/amd64
go version devel +c11f6c4929 Fri Jun 21 21:24:47 2019 +0000 linux/amd64

What did you do?

https://play.golang.org/p/Y0ANYGkISpa

What did you expect to see?

Two packages, sandbox/empty1 and sandbox/empty2, each with one error

What did you see instead?

One package, sandbox/empty2, with one error but whose error message contains two errors for two separate packages.

$ go run foo.go
2019/06/24 20:22:59 sandbox/empty2
2019/06/24 20:22:59 1
2019/06/24 20:22:59 packages.Error{Pos:"", Msg:"go build sandbox/empty2: no Go files in /home/dominikh/prj/src/sandbox/empty2\ngo build sandbox/empty1: no Go files in /home/dominikh/prj/src/sandbox/empty1", Kind:0}

/cc @matloob

@dominikh dominikh added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Jun 24, 2019
@dominikh dominikh added this to the Unreleased milestone Jun 24, 2019
@dominikh
Copy link
Member Author

The Mode can be further reduced to packages.NeedImports | packages.NeedExportsFile and still reproduce the bug.

The underlying go list call seems to behave correctly:

$ go list -e -json -compiled=false -test=false -export=true -deps=false -find=false -- sandbox/foo
go build sandbox/empty2: no Go files in /home/dominikh/prj/src/sandbox/empty2
go build sandbox/empty1: no Go files in /home/dominikh/prj/src/sandbox/empty1
{
	"Dir": "/home/dominikh/prj/src/sandbox/foo",
	"ImportPath": "sandbox/foo",
	"Name": "pkg",
	"Target": "/home/dominikh/prj/pkg/linux_amd64/sandbox/foo.a",
	"Root": "/home/dominikh/prj/",
	"Match": [
		"sandbox/foo"
	],
	"Incomplete": true,
	"GoFiles": [
		"foo.go"
	],
	"Imports": [
		"sandbox/empty1",
		"sandbox/empty2"
	],
	"Deps": [
		"sandbox/empty1",
		"sandbox/empty2"
	],
	"DepsErrors": [
		{
			"ImportStack": [
				"sandbox/foo",
				"sandbox/empty1"
			],
			"Pos": "/home/dominikh/prj/src/sandbox/foo/foo.go:3:8",
			"Err": "no Go files in /home/dominikh/prj/src/sandbox/empty1"
		},
		{
			"ImportStack": [
				"sandbox/foo",
				"sandbox/empty2"
			],
			"Pos": "/home/dominikh/prj/src/sandbox/foo/foo.go:4:8",
			"Err": "no Go files in /home/dominikh/prj/src/sandbox/empty2"
		}
	]
}

though the extra output on stderr is noteworthy.

@dominikh
Copy link
Member Author

Worse yet, if we attempt to load a healthy package in addition to sandbox/foo – e.g. packages.Load(cfg, "sandbox/foo", "strconv") – then the healthy package will not be returned by packages.Load. We still only get exactly one package, sandbox/empty2, with exactly one packages.Error.

@sauyon
Copy link

sauyon commented Jul 2, 2019

I'm reasonably sure that this is caused by this special case in the golist driver.

This might be a bug on the older go1.1 version which I assume is supported (?) but doesn't seem to be an issue on my version (go1.12.6).

Either way, I think the correct way to correct this is to add special JSON nodes to stdout, then, when parsing, remove them if there is already a node for them.

sauyon added a commit to sauyon/tools that referenced this issue Jul 4, 2019
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
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
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
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
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
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
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
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 30, 2019

This seems to have been fixed on head. Please reopen if not.

Running with the same gopath layout on Go 1.13 with GO111MODULE=off, and the following test code:

package main

import (
	"log"

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

func main() {
	cfg := &packages.Config{
		Mode: packages.NeedName | packages.NeedImports | packages.NeedDeps | packages.NeedExportsFile | packages.NeedFiles | packages.NeedCompiledGoFiles | packages.NeedTypesSizes,
	}
	pkgs, err := packages.Load(cfg, "sandbox/foo")
	if err != nil {
		log.Println(err)
	}
	for _, pkg := range pkgs {
		log.Println(pkg)
		for _, err := range pkg.Errors {
			log.Printf("%#v", err)
		}
		for _, pkg := range pkg.Imports {
			log.Println(pkg)
			for _, err := range pkg.Errors {
				log.Printf("%#v", err)
			}
		}
	}
}

produces the following output for me:

2019/10/29 20:47:05 sandbox/foo
2019/10/29 20:47:05 sandbox/empty2
2019/10/29 20:47:05 packages.Error{Pos:"src/sandbox/foo/foo.go:4:8", Msg:"no Go files in /Users/matloob/Desktop/gopath/src/sandbox/empty2", Kind:0}
2019/10/29 20:47:05 sandbox/empty1
2019/10/29 20:47:05 packages.Error{Pos:"src/sandbox/foo/foo.go:3:8", Msg:"no Go files in /Users/matloob/Desktop/gopath/src/sandbox/empty1", Kind:0}

@matloob matloob closed this as completed Oct 30, 2019
@golang golang locked and limited conversation to collaborators Oct 29, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
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
Development

No branches or pull requests

4 participants