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/cmd/goimports: GOPATH not honored with absolute file paths #16458

Closed
akavel opened this issue Jul 21, 2016 · 5 comments
Closed

x/tools/cmd/goimports: GOPATH not honored with absolute file paths #16458

akavel opened this issue Jul 21, 2016 · 5 comments

Comments

@akavel
Copy link
Contributor

@akavel akavel commented Jul 21, 2016

Please answer these questions before submitting your issue. Thanks!

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

go version go1.5.4 linux/amd64

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

GOARCH="amd64"
GOBIN=""
GOEXE=""
GOHOSTARCH="amd64"
GOHOSTOS="linux"
GOOS="linux"
GOPATH="/home/akavel/gopath/src/zpaslab.com/lockerbox/_vendor:/home/akavel/gopath"
GORACE=""
GOROOT="/home/akavel/go"
GOTOOLDIR="/home/akavel/go/pkg/tool/linux_amd64"
GO15VENDOREXPERIMENT=""
CC="gcc"
GOGCCFLAGS="-fPIC -m64 -pthread -fmessage-length=0"
CXX="g++"
CGO_ENABLED="1"

  1. What did you do?
    If possible, provide a recipe for reproducing the error.
    A complete runnable program is good.
    A link on play.golang.org is best.

I've upgraded goimports, following the recent "it's now faster" announcement. I retained the old binary, and now I have 2 binaries:

$ LC_ALL=C ll ~/gopath/bin/goimports*
-rwxr-xr-x 1 akavel akavel 5509320 Jul 21 13:59 /home/akavel/gopath/bin/goimports*
-rwxr-xr-x 1 akavel akavel 5323816 Apr 27 14:49 /home/akavel/gopath/bin/goimports.old1*
-rwxr-xr-x 1 akavel akavel 5509320 Jul 21 13:59 /home/akavel/gopath/bin/goimports.old2*
$ LC_ALL=C date -Isec
2016-07-21T14:21:05+0200                                            

Depending whether I run the new one with relative or absolute path to source file, import gets found or not:

$ goimports.old2 /home/akavel/gopath/src/zpaslab.com/lockerbox/overmind/collector_rest_test.go | grep protobuf
$ goimports.old1 /home/akavel/gopath/src/zpaslab.com/lockerbox/overmind/collector_rest_test.go | grep protobuf
        "github.com/golang/protobuf/proto"
$ goimports.old2 ./overmind/collector_rest_test.go | grep protobuf
        "github.com/golang/protobuf/proto"
$ goimports.old1 ./overmind/collector_rest_test.go | grep protobuf
        "github.com/golang/protobuf/proto"

Additional notes:

  • exactly the same behavior happens for packages not in _vendor/, e.g. "zpaslab.com/lockerbox/db"
  • standard library packages are found OK (e.g. "testing")
  • the same seems to happen when using -srcdir (i.e. in vim-go)
  1. What did you expect to see?

All versions would find the import:

$ goimports.old2 /home/akavel/gopath/src/zpaslab.com/lockerbox/overmind/collector_rest_test.go | grep protobuf
        "github.com/golang/protobuf/proto"
$ goimports.old1 /home/akavel/gopath/src/zpaslab.com/lockerbox/overmind/collector_rest_test.go | grep protobuf
        "github.com/golang/protobuf/proto"
$ goimports.old2 ./overmind/collector_rest_test.go | grep protobuf
        "github.com/golang/protobuf/proto"
$ goimports.old1 ./overmind/collector_rest_test.go | grep protobuf
        "github.com/golang/protobuf/proto"
  1. What did you see instead?
$ goimports.old2 /home/akavel/gopath/src/zpaslab.com/lockerbox/overmind/collector_rest_test.go | grep protobuf
$ goimports.old1 /home/akavel/gopath/src/zpaslab.com/lockerbox/overmind/collector_rest_test.go | grep protobuf
        "github.com/golang/protobuf/proto"
$ goimports.old2 ./overmind/collector_rest_test.go | grep protobuf
        "github.com/golang/protobuf/proto"
$ goimports.old1 ./overmind/collector_rest_test.go | grep protobuf
        "github.com/golang/protobuf/proto"
@bradfitz
Copy link
Contributor

@bradfitz bradfitz commented Jul 21, 2016

What does the -v flag to goimports.old2 say with an absolute path?

$ goimports.old2 -v /home/akavel/gopath/src/zpaslab.com/lockerbox/overmind/collector_rest_test.go

And can you diff the -v output between the absolute and relative path mode and include that too?

@bradfitz bradfitz added this to the Unreleased milestone Jul 21, 2016
@stapelberg
Copy link
Contributor

@stapelberg stapelberg commented Jul 24, 2016

I’m facing the same issue.

git bisect identifies commit golang/tools@e047ae7 as the first bad commit

Unfortunately, the previous commit does not have the -v flag, so I can’t provide the output you’ve asked for.

However, while collecting the steps to reproduce, I think I figured out what caused the issue for me:

$ go env GOPATH GOROOT
/home/michael/gocode
/home/michael/go

The following change makes goimports work again in my setup:

diff --git i/imports/fix.go w/imports/fix.go
index 4d7dfeb..ca721e0 100644
--- i/imports/fix.go
+++ w/imports/fix.go
@@ -661,7 +661,7 @@ func findImportGoPath(pkgName string, symbols map[string]bool, filename string)
        // TODO(bradfitz): run each $GOPATH entry async. But nobody
        // really has more than one anyway, so low priority.
        scanGoRootOnce.Do(scanGoRoot) // async
-       if !strings.HasPrefix(filename, build.Default.GOROOT) {
+       if !strings.HasPrefix(filename, build.Default.GOROOT+"/") {
                scanGoPathOnce.Do(scanGoPath) // blocking
        }
        <-scanGoRootDone
@bradfitz
Copy link
Contributor

@bradfitz bradfitz commented Jul 25, 2016

@stapelberg, thanks for the clue. I sent out https://golang.org/cl/25192. Feel free to review.

@gopherbot
Copy link

@gopherbot gopherbot commented Jul 25, 2016

CL https://golang.org/cl/25192 mentions this issue.

@akavel
Copy link
Contributor Author

@akavel akavel commented Jul 26, 2016

the CL fixes it, thanks guys! (i.e. works correctly in vim-go now)

@golang golang locked and limited conversation to collaborators Jul 26, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
4 participants
You can’t perform that action at this time.