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

Jump to bottom

x/tools/go/packages: Imports from GOPACKAGESDRIVERs are not handled correctly #34851

Closed
jmhodges opened this issue Oct 11, 2019 · 6 comments
Closed

x/tools/go/packages: Imports from GOPACKAGESDRIVERs are not handled correctly #34851

jmhodges opened this issue Oct 11, 2019 · 6 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
Unreleased

Comments

@jmhodges
Copy link
Contributor

jmhodges commented Oct 11, 2019 •
edited

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

$ go version
go version go1.13.1 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

GO111MODULE=""

GOARCH="amd64"

GOBIN=""

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

GOENV="/Users/jeffhodges/Library/Application Support/go/env"

GOEXE=""

GOFLAGS=""

GOHOSTARCH="amd64"

GOHOSTOS="darwin"

GONOPROXY=""

GONOSUMDB=""

GOOS="darwin"

GOPATH="/Users/jeffhodges"

GOPRIVATE=""

GOPROXY="https://proxy.golang.org,direct"

GOROOT="/usr/local/Cellar/go/1.13.1/libexec"

GOSUMDB="sum.golang.org"

GOTMPDIR=""

GOTOOLDIR="/usr/local/Cellar/go/1.13.1/libexec/pkg/tool/darwin_amd64"

GCCGO="gccgo"

AR="ar"

CC="clang"

CXX="clang++"

CGO_ENABLED="1"

GOMOD="/Users/jeffhodges/src/golang.org/x/tools/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/vg/32jn4gdd1x3f_r9dwcryw4y00000gn/T/go-build399318009=/tmp/go-build -gno-record-gcc-switches -fno-common"

What did you do?

While writing a GOPACKAGESDRIVER, I ran across a problem where my Imports, keyed by package ID and where the ID and the PkgPath are different because of the underlying build system, are being dropped inside packages.Load

I've produced a reproduction in a fork of x/tools in the simpledriver branch of my fork: https://github.com/jmhodges/tools/tree/simpledriver

You can reproduce it by running go test . -run TestDriverGoldenPath/2 (in the go/packages directory). My faked out simpledriver binary returns Imports maps keyed by the package IDs, but the Imports map returned by packages.Load is empty.

Perhaps I'm missing something?

What did you expect to see?

The Imports of Packages returned from packages.Load should have the Imports in them that are being outputted by the simpledriver code and keyed by package ID.

What did you see instead?

The Imports maps are empty even though the simpledriver returns seemingly correct ones.
@gopherbot gopherbot added this to the Unreleased milestone Oct 11, 2019
@gopherbot gopherbot added the Tools This label describes issues relating to any tools in the x/tools repository. label Oct 11, 2019
@jmhodges jmhodges mentioned this issue Oct 11, 2019
Closed
@katiehockman katiehockman added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Oct 14, 2019
@katiehockman
Copy link
Contributor

/cc @matloob

@jmhodges
Copy link
Contributor Author

Is there anything I can do to help debug this?

I've rebased my reproduction and this bug still exists. https://github.com/jmhodges/tools/tree/simpledriver

Am I right in thinking that Package.Imports is to be keyed by Package.ID and not Package.ImportPath?

@jmhodges
Copy link
Contributor Author

jmhodges commented Oct 22, 2019 •
edited

Hm, keying by ImportPath doesn't seem to do it, either.

I'm not sure what assumption I'm getting wrong here.

I pushed up the version of my repro that keys by ImportPaths instead. You can roll back one commit to get back to the ID version.

@jmhodges
Copy link
Contributor Author

Hm, okay, is it that the GOPACKAGESDRIVER is supposed to be returning all packages referenced in Imports in the slice of Packages in the driverResponse, as well? I was under the impression that those were only for roots.

That would explain this

@jmhodges
Copy link
Contributor Author

Yep, that's it. Welp.

@gopherbot
Copy link

Change https://golang.org/cl/207617 mentions this issue: go/packages: add tests of GOPACKAGESDRIVER integration

@golang golang locked and limited conversation to collaborators Nov 17, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
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
Milestone
Unreleased
Development

No branches or pull requests
3 participants
@jmhodges @gopherbot @katiehockman