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: support vendor directories #13644

Closed
cespare opened this issue Dec 16, 2015 · 12 comments
Closed

x/tools/cmd/goimports: support vendor directories #13644

cespare opened this issue Dec 16, 2015 · 12 comments

Comments

@cespare
Copy link
Contributor

cespare commented Dec 16, 2015

#12278 was originally about this, but was changed to be about go/build.

go/build and x/tools/go/loader now know about vendor directories, but there is more work to do to make goimports add the correct paths.

When working on $GOPATH/src/a that vendors b, instead of adding the (disallowed) import "a/vendor/b", goimports should add "b".

@FiloSottile
Copy link
Contributor

@rsc rsc added this to the Unreleased milestone Dec 28, 2015
fsouza pushed a commit to tsuru/tsuru that referenced this issue Jan 6, 2016
fsouza pushed a commit to tsuru/gandalf that referenced this issue Jan 7, 2016
@termie
Copy link

termie commented Jan 14, 2016

excited about this feature landing :) (trying to move a bunch of code to subpackages)

@termie
Copy link

termie commented Jan 14, 2016

just mirroring here in case people don't look at the gerrit review, here's a patch on top of that review that gets goimports and vim-go working together again https://gist.github.com/termie/34326bd29e89565c95fa

@ottob
Copy link

ottob commented Feb 6, 2016

Thanks for the patch @termie it works fine for me.

@pwaller
Copy link
Contributor

pwaller commented Feb 8, 2016

This may be a duplicate of #10339.

@cespare
Copy link
Contributor Author

cespare commented Feb 9, 2016

@pwaller not quite. #10339 predates the vendoring support in the Go tool; it is about choosing "github.com/a/b/vendor/github.com/c/d" over "github.com/c/d". I would say that #10339 as written is obviated by GO15VENDOREXPERIMENT and this issue.

Once CL 17728 is merged both this issue and #10339 should probably be closed.

@zellyn
Copy link

zellyn commented Mar 2, 2016

I think this is fixed by https://go-review.googlesource.com/#/c/17728/

@cezarsa
Copy link
Contributor

cezarsa commented Apr 13, 2016

Regarding this issue I think I found another code path that wasn't considering packages inside /vendor and created a new CL fixing it: https://go-review.googlesource.com/#/c/22020/

The problem happens when the vendored package name is different from the base import path. goimports calls build.Import() to figure out the real package name, however, it was being called with an empty srcDir parameter this way it couldn't find packages inside /vendor.

@favadi
Copy link

favadi commented May 9, 2016

@cezarsa what else need to be done to have your patch merged?

@ricardoseriani
Copy link

Please, any news on this ?

@FiloSottile
Copy link
Contributor

FiloSottile commented Jun 10, 2016 via email

@cespare
Copy link
Contributor Author

cespare commented Jun 21, 2016

This was fixed some time ago.

@cespare cespare closed this as completed Jun 21, 2016
@golang golang locked and limited conversation to collaborators Jun 21, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests