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 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".

@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
Francisco Souza
fsouza pushed a commit to tsuru/gandalf that referenced this issue Jan 7, 2016
Also disabling goimports for now. See golang/go#13644.
@termie
Copy link

@termie termie commented Jan 14, 2016

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

@termie
Copy link

@termie 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 ottob commented Feb 6, 2016

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

@pwaller
Copy link
Contributor

@pwaller pwaller commented Feb 8, 2016

This may be a duplicate of #10339.

@cespare
Copy link
Contributor Author

@cespare 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 zellyn commented Mar 2, 2016

@bradfitz bradfitz added the Vendoring label Apr 10, 2016
@cezarsa
Copy link
Contributor

@cezarsa 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 favadi commented May 9, 2016

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

@ricardoseriani
Copy link

@ricardoseriani ricardoseriani commented Jun 10, 2016

Please, any news on this ?

@FiloSottile
Copy link
Member

@FiloSottile FiloSottile commented Jun 10, 2016

@cespare
Copy link
Contributor Author

@cespare cespare commented Jun 21, 2016

This was fixed some time ago.

@cespare cespare closed this 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
Linked pull requests

Successfully merging a pull request may close this issue.

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