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.

Sign up for GitHub

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/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
Labels
FrozenDueToAge WaitingForInfo Issue is not actionable because of missing required information, which needs to be provided.
Milestone
Unreleased

Comments

@akavel
Copy link
Contributor

akavel commented Jul 21, 2016 •
edited

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

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
@bradfitz bradfitz added the WaitingForInfo Issue is not actionable because of missing required information, which needs to be provided. label Jul 21, 2016
@stapelberg
Copy link
Contributor

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

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

@gopherbot
Copy link

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

@akavel
Copy link
Contributor Author

akavel commented Jul 26, 2016 •
edited

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.
Assignees
No one assigned
Labels
FrozenDueToAge WaitingForInfo Issue is not actionable because of missing required information, which needs to be provided.
Projects
None yet
Milestone
Unreleased
Development

No branches or pull requests
4 participants
@bradfitz @stapelberg @akavel @gopherbot