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: Load in NeedName|NeedFiles mode can produce incorrect results for certain modules #31894

Closed
dmitshur opened this issue May 7, 2019 · 1 comment
Labels
FrozenDueToAge modules NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Milestone

Comments

@dmitshur
Copy link
Contributor

dmitshur commented May 7, 2019

This issue is related to #29452, #31087, and #31893, but is more narrowly focused on the correctness of the packages.NeedName | packages.NeedFiles mode.

Background

Both go/packages and go/build are concerned with loading Go packages and returning information about them.

Go packages may be inside modules that are tidy and compile successfully, or they may be incomplete and fail to build in certain configurations. Loading information about a package should succeed independently from whether or not building said package (in a specific configuration) can be done without build errors.

Environment

$ go version
go version go1.12.5 darwin/amd64

Issue

Incorrect results are produced when using the go/packages package in packages.NeedName | packages.NeedFiles mode. Correct results are produced by the go/build package.

To reproduce the issue, create a target package to be imported in a temporary directory /tmp/target with the following files:

-- go.mod --
module m

go 1.12

require github.com/google/go-github v0.0.0-20181014183319-aa7423eb7970

-- main.go --
// +build ignore

package main

func main() {}

-- p.go --
package p

import "github.com/google/go-github/github"

var _ *github.Client

Then, inside another directory, create the following test command:

-- go.mod --
module m

go 1.12

require golang.org/x/tools v0.0.0-20190506145303-2d16b83fe98c

-- main.go --
package main

import (
	"fmt"
	"go/build"

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

func main() {
	for driver, f := range map[string]func(dir string) (name string, goFiles []string, _ error){
		"go/build":    useGoBuild,
		"go/packages": useGoPackages,
	} {
		name, goFiles, error := f("/tmp/target")

		fmt.Println(driver)
		fmt.Println("  error:", error)
		if error == nil {
			fmt.Println("  name:", name)
			fmt.Println("  goFiles:", goFiles)
		}
		fmt.Println()
	}
}

func useGoBuild(dir string) (name string, goFiles []string, _ error) {
	p, err := build.ImportDir(dir, 0)
	if err != nil {
		return "", nil, fmt.Errorf("build.ImportDir: %v", err)
	}
	return p.Name, p.GoFiles, nil
}

func useGoPackages(dir string) (name string, goFiles []string, _ error) {
	cfg := &packages.Config{
		Mode: packages.NeedName | packages.NeedFiles,
		Dir:  dir,
	}
	pkgs, err := packages.Load(cfg, ".")
	if err != nil {
		return "", nil, fmt.Errorf("packages.Load: %v", err)
	}
	if len(pkgs) != 1 {
		return "", nil, fmt.Errorf("got %d packages, want exactly 1 package", len(pkgs))
	}
	p := pkgs[0]
	if len(p.Errors) > 0 {
		return "", nil, fmt.Errorf("p.Errors: %v", p.Errors)
	}
	return p.Name, p.GoFiles, nil
}

Run it as follows:

$ export GO111MODULE=on
$ export GOPROXY=direct
$ export GOPATH=$(mktemp -d)
$ go build -o /tmp/loadpkg
$ /tmp/loadpkg
go/build
  error: <nil>
  name: p
  goFiles: [p.go]

go/packages
  error: packages.Load: go [list -e -json -compiled=false -test=false -export=false -deps=false -find=true -- .]: exit status 1: go: github.com/google/go-github@v0.0.0-20181014183319-aa7423eb7970: go.mod has post-v0 module path "github.com/google/go-github/v18" at revision aa7423eb7970
go: error loading module requirements

Notice that go/build produced correct results, while go/packages produced incorrect results (an error).

/cc @matloob per owners.

@dmitshur dmitshur added NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. modules labels May 7, 2019
@dmitshur dmitshur added this to the Unreleased milestone May 7, 2019
@dmitshur dmitshur changed the title x/tools/go/packages: Load in NeedName|NeedFiles can produce incorrect results for certain modules x/tools/go/packages: Load in NeedName|NeedFiles mode can produce incorrect results for certain modules May 10, 2019
@matloob
Copy link
Contributor

matloob commented Jun 19, 2019

Unfortunately this is one of those cases where we're bound by what go list does. If go list can successfully produce a package, then go/packages will be able to also. But this would require us to pass extra information about what we want to go list.

In any case, as things are now, this is infeasible to fix.

@matloob matloob closed this as completed Jun 19, 2019
@golang golang locked and limited conversation to collaborators Jun 18, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
FrozenDueToAge modules NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Projects
None yet
Development

No branches or pull requests

3 participants