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: does not find import with package in vendor/, GOFLAGS="-mod=vendor" and empty $GOPATH/pkg directory #32084

Open
fho opened this issue May 16, 2019 · 5 comments

Comments

@fho
Copy link

@fho fho commented May 16, 2019

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

$ go version
go version go1.12.4

goimports from from https://github.com/golang/tools, commit d1a3278ee74994e9aa609e9e711c616bba677d5d

Does this issue reproduce with the latest release?

Yes, reproducible with go 1.12.5

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

GOARCH="amd64"
GOBIN=""
GOCACHE="/home/fho/.cache/go-build"
GOEXE=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="linux"
GOOS="linux"
GOPATH="/home/fho/go"
GOPROXY=""
GORACE=""
GOROOT="/usr/lib/go"
GOTMPDIR=""
GOTOOLDIR="/usr/lib/go/pkg/tool/linux_amd64"
GCCGO="gccgo"
CC="gcc"
CXX="g++"
CGO_ENABLED="1"
GOMOD="/tmp/testproject/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 -fmessage-length=0 -fdebug-prefix-map=/tmp/go-build188015067=/tmp/go-build -gno-record-gcc-switches"

What did you do?

  1. Create a new golang project using go.mod.
    The project has a a main.go file that imports the external dependency github.com/lib/pq.
    The external modules are downloaded into vendor/ via go mod vendor.
mkdir -p /tmp/testproject
cd /tmp/testproject
go mod init "testproject"
echo 'package main

import (
	"fmt"
	"github.com/lib/pq"
)

func main() {
	var test pq.StringArray

	fmt.Println(test)
}' > main.go
go mod vendor
  1. Remove the "github.com/lib/pq" import from main.go:
sed -i '/"github.com\/lib\/pq"/D' main.go
  1. Remove $GOPATH/pkg, run goimports with GOFLAGS=-mod=vendor:
sudo rm -rf $(go env GOPATH)/pkg
GOFLAGS="-mod=vendor" goimports -d main.go

What did you expect to see?

goimports detects the missing github.com/lib/pq import and shows it in the diff.

What did you see instead?

goimports does not find the missing github.com/lib/pg import and prints no diff.

@gopherbot gopherbot added this to the Unreleased milestone May 16, 2019
@fho fho changed the title x/tools/cmd/goimports: does not find import with package in `vendor/`, `GOFLAGS="-mod=vendor"` and empty `$GOPATH/pkg` directory x/tools/cmd/goimports: does not find import with package in vendor/, GOFLAGS="-mod=vendor" and empty $GOPATH/pkg directory May 16, 2019
@heschik

This comment has been minimized.

Copy link
Contributor

@heschik heschik commented May 16, 2019

Thanks for the excellent repro. The immediate problem is that there's no structured way for goimports to tell if vendoring is enabled. From https://github.com/golang/tools/blob/d1a3278ee74994e9aa609e9e711c616bba677d5d/imports/mod.go#L241-L248:

		if strings.HasPrefix(importPath, "vendor/") {
			// Ignore vendor dirs. If -mod=vendor is on, then things
			// should mostly just work, but when it's not vendor/
			// is a mess. There's no easy way to tell if it's on.
			// We can still find things in the mod cache and
			// map them into /vendor when -mod=vendor is on.
			return
		}

Possibly we should just attempt to parse $GOFLAGS. I think everything will work after that.

@heschik

This comment has been minimized.

Copy link
Contributor

@heschik heschik commented Nov 7, 2019

I fixed this in https://golang.org/cl/203017.

@heschik heschik closed this Nov 7, 2019
@fho

This comment has been minimized.

Copy link
Author

@fho fho commented Nov 8, 2019

thanks a lot @heschik

@alexshkatov

This comment has been minimized.

Copy link

@alexshkatov alexshkatov commented Dec 16, 2019

The issue still persists on go1.13.1 (exactly the same scenario) . It seems like the latest goimports tool ignores GOFLAGS completely.

@heschik heschik reopened this Dec 16, 2019
@heschik

This comment has been minimized.

Copy link
Contributor

@heschik heschik commented Dec 16, 2019

You're right. Sorry about that; the unit test coverage was good but I missed that GOFLAGS wasn't hooked up in the actual binary.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

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