go/build: Import doesn't work as expected with modules

[dimes]

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

$ go version
go version go1.12.4 darwin/amd64

Does this issue reproduce with the latest release?

Yes

What operating system and processor architecture are you using (go env)?

go env Output

$ go env
GOARCH="amd64"

GOBIN=""

GOCACHE="/Users/maxgale/Library/Caches/go-build"

GOEXE=""

GOFLAGS=""

GOHOSTARCH="amd64"

GOHOSTOS="darwin"

GOOS="darwin"

GOPATH="/Users/maxgale/go"

GOPROXY=""

GORACE=""

GOROOT="/usr/local/go"

GOTMPDIR=""

GOTOOLDIR="/usr/local/go/pkg/tool/darwin_amd64"

GCCGO="gccgo"

CC="clang"

CXX="clang++"

CGO_ENABLED="1"

GOMOD="/Users/maxgale/Dev/my-test-package/go.mod"

CGO_CFLAGS="-g -O2"

CGO_CPPFLAGS=""

CGO_CXXFLAGS="-g -O2"

CGO_FFLAGS="-g -O2"

CGO_LDFLAGS="-g -O2"

PKG_CONFIG="pkg-config"

GOGCCFLAGS="-fPIC -m64 -pthread -fno-caret-diagnostics -Qunused-arguments -fmessage-length=0 -fdebug-prefix-map=/var/folders/pd/4cgwkjzx609_9yb4xdb2lfp80000gp/T/go-build151436703=/tmp/go-build -gno-record-gcc-switches -fno-common"

What did you do?

	packageName := "github.com/dimes/my-test-package"
	imported, err := build.Default.Import(packageName, ".", build.FindOnly)
	if err != nil {
		return nil, errors.Wrapf(err, "Error importing package %s", packageName)
	}

	packages, err := parser.ParseDir(fileSet, imported.Dir, nil, 0)
	if err != nil {
		return nil, errors.Wrapf(err, "Error parsing package %s", packageName)
	}

	for _, astPkg := range packages {
		var files []*ast.File
		for _, file := range astPkg.Files {
			files = append(files, file)
		}

		info := &types.Info{
			Defs: make(map[*ast.Ident]types.Object),
		}

		conf := types.Config{
			Importer: importer.ForCompiler(fileSet, "source", nil),
		}

		_, err := conf.Check(packageName, fileSet, files, info)
		if err != nil {
			panic(err) // Panics here
		}
	}

What did you expect to see?

I expected no error from conf.Check

What did you see instead?

panic: /Users/maxgale/Dev/my-test-package/bindings/bindings.go:6:2: could not import github.com/dimes/my-test-package/prototype (type-checking package "github.com/dimes/my-test-package/prototype" failed (/Users/maxgale/Dev/my-test-package/prototype/basic.go:3:8: could not import <omitted> could not import github.com/golang/protobuf/ptypes/wrappers (type-checking package "github.com/golang/protobuf/ptypes/wrappers" failed (/Users/maxgale/go/pkg/mod/github.com/golang/protobuf@v1.2.0/ptypes/wrappers/wrappers.pb.go:6:14: could not import github.com/golang/protobuf/proto (cannot find package "github.com/golang/protobuf/proto" in any of:
	/usr/local/go/src/github.com/golang/protobuf/proto (from $GOROOT)
	/Users/maxgale/go/src/github.com/golang/protobuf/proto (from $GOPATH))))))))))))))))

However, if from the command line, I run go list -f '{{ .Dir }}' github.com/golang/protobuf/proto I get /Users/maxgale/go/pkg/mod/github.com/golang/protobuf@v1.3.1/proto. Another peculiarity is that the omitted packages are pulled from Github Enterprise repos.

[katiehockman]

/cc @griesemer

[dimes]

@griesemer can you look at this issue? I have a very simple repo case. Basically, if a package in a go module depends on a package that has a vendor directory, conf.Check returns an error. Maybe the packages aren't returned from parser.ParseDir?

[griesemer]

@dimes. Ok, will look at this next week.

[dimes]

Thanks. I created a really simple example. Here is a package with a vendor directory: https://github.com/dimes/testtesttest

You can repro this by creating a new Go module:

1. go mod init some/package
2. go get github.com/dimes/testtesttest
3. Create some file that imports the package:

package mypkg;

import "github.com/dimes/testtesttest"

type SomeInterface interface {
	SomeFunction() testtesttest.FooBar
}

4. In the example in the first issue post, set packageName to some/package (the input to go mod init).

[griesemer]

I cannot reproduce this issue.

Here's what I have:

• $GOPATH/src/github.com/dimes contains your testtesttest package incl. the vendored directory.
• $GOPATH/src/mypkg contains mypkg.go
• I used the following package containing your code from your original comment: x.go (in $HOME/tmp).
• I'm running it via go run x.go in $HOME/tmp.

The x.go package traces various things, including imports. I'm seeing:

$ go run x.go
imported.Dir = /Users/gri/src/mypkg
package mypkg
    - /Users/gri/src/mypkg/mypkg.go
importing "github.com/dimes/testtesttest" (dir = /Users/gri/src/mypkg, mode = 0) => package testtesttest ("github.com/dimes/testtesttest") (err = <nil>)

You are using import.ForCompiler(fileSet, "source", nil). The "source" mode requires that all source files are present, recursively.
Are you sure all the source .go files are present? (Looking at your error message, do all those files exist in your file system?)

More generally: The parser.ParseDir function is ancient and from a time before we had go/build. Now that go/build provides a build.Package, you should use the files from that Package, parse those individually (via parser.ParseFile) and then provide them to types.Config.Check (which is already built to only accept a list of files. The ast.Packages returned by ParseDir do not have complete information; for instance, they ignore +build tags (though this doesn't matter in this case).

Please double-check your situation and reply. Thanks.

[dimes]

Thanks @griesemer. I will try using build.Package now. One difference between our two setups is that you have your $GOPATH set and my testtesttest package inside. I did not have a $GOPATH set and was using go mod to both fetch testtesttest and run the code. Sorry for omitting that.

[dimes]

@griesemer I just tried using build.Package in the following way:

	var files []*ast.File
	imported, err := build.Default.Import(packageName, ".", 0)
	if err != nil {
		return nil, errors.Wrapf(err, "Error importing package %s", packageName)
	}

	for _, goFile := range imported.GoFiles {
		fileName := filepath.Join(imported.Dir, goFile)
		fmt.Println("Parsing", fileName)
		file, err := parser.ParseFile(fileSet, fileName, nil, parser.AllErrors)
		if err != nil {
			return nil, errors.Wrapf(err, "Error parsing go file %s", fileName)
		}
		files = append(files, file)
	}

	info := &types.Info{
		Defs: make(map[*ast.Ident]types.Object),
	}

	conf := types.Config{
		Importer: importer.ForCompiler(fileSet, "source", nil),
	}

	_, err = conf.Check(packageName, fileSet, files, info) // err is non-nil

The error is the same. I think there is a weird interaction here when using Go modules.

[dimes]

@griesemer sorry for the spam, but I believe I figured out the root cause of this issue. Inside of Checker.collectObjects, the imports are extracted from the files and then imported here: https://golang.org/src/go/types/resolver.go#L256

The issue seems to be that:

check.importPackage(s.Path.Pos(), path, fileDir)

Is using fileDir as the directory to do the import from. Internally, check.importPackage uses the provided Importer to do the import. When the $GOPATH is set, this is fine. However, when using go mod, the importer looks for a go.mod file in the srcDir given to Import call. The fileDir of the dependency does not have a go.mod file, so it falls back to using the $GOPATH. It does not find the package. If I change importPackage call in resolver.go to be:

check.importPackage(s.Path.Pos(), path, ".") // go.mod located at "./go.mod" 

then it works. Not sure what the general solution to this is.

[griesemer]

@dimes Thanks for this analysis, that is very helpful. Looping in @bcmills who has been working on module support for additional input.

(The fileDir argument provided to importPackage was added in the past to deal with vendor directories since vendored packages have to be lookup up relative to the directory of the package that is doing the import. This has all changed with modules. It may be that the importer implementations may have to be updated.)

[bcmills]

CC @matloob @ianthehat