Skip to content
This repository was archived by the owner on May 9, 2021. It is now read-only.

add support for GO15VENDOREXPERIMENT #151

Closed
toxeus opened this issue Jul 31, 2015 · 13 comments
Closed

add support for GO15VENDOREXPERIMENT #151

toxeus opened this issue Jul 31, 2015 · 13 comments
Assignees

Comments

@toxeus
Copy link

toxeus commented Jul 31, 2015

For background see here and here. Therefore, golint will blindly check vendored dependencies when GO15VENDOREXPERIMENT=1. Since we cannot expect all dependencies to be golint compliant the vendor folder should be ignored by golint when GO15VENDOREXPERIMENT=1.

@dsymonds
Copy link
Contributor

dsymonds commented Aug 2, 2015

I'm not sure ignoring is right. The go tool doesn't ignore vendored packages if you use it with a ./... argument; why should golint?

@toxeus
Copy link
Author

toxeus commented Aug 3, 2015

why should golint?

I think that it's pretty sane to assume that your dependencies should not fail on go build and go test and therefore not ignore the dependencies when specifying ./... for the go tool.

For golint it's a different story because - as of now - most go packages are not golint compliant. IMHO it would be the wrong thing to punish a project that tries to be golint compliant by making it work around the dependencies' golint errors.

Alternatively, a -ignore flag could be provided. But is certainly much less convenient.

@brocaar
Copy link

brocaar commented Aug 11, 2015

@adg proposed to use this workaround for the go tool (golang/go#11659):

go install $(go list ./... | grep -v /vendor/)

Maybe we could make this work for the golint tool too (golint $(go list ./... |grep -v /vendor/))?
It seems that now the golint tool parses the arguments provided by go list ./... as files and thus resulting in the error no such file or directory for each package.

My current workaround (in a Makefile) is:

test:
    for pkg in $$(go list ./... |grep -v /vendor/) ; do \
        golint $$pkg ; \
    done

    # other tests

@ascarter
Copy link

In my opinion, the go tool should be ignoring vendor - to me this code should be exactly like library code in your workspace, just in the vendor/. I think vendor is a snapshot that most people would want to leave untouched. If you did want to locally fork in vendor, explicitly providing vendor/path/to/thing seems perfectly fine. I'm betting the 90% case is like what @toxeus, @brocaar, and I are experiencing.

Adding ignore option would be an ok workaround. @brocaar - thanks for the workaround - that unblocks our CI for the time being.

@ifraixedes
Copy link

I agree with @ascarter
Many thanks @brocaar for the workaround

@dsymonds
Copy link
Contributor

golang/go#12278 is probably going to be the right solution. Every tool that operates on Go packages shouldn't hack in their own mechanism for handling vendored packages.

@fiorix
Copy link

fiorix commented Oct 23, 2015

@brocaar you can do that with a one-liner: go list ./... | grep -v /vendor/ | xargs -L1 cmd where cmd is golint or whatever.

@paddycarver
Copy link

Just wanted to chime in that this is currently kind of non-obvious, and a workaround being documented better (or supporting the same workaround used for the go command) would be a great step forward, in my opinion. I have some thoughts about how to add support in a non-breaking-change way, but none are particularly elegant (add it as a flag option, allow multiple package paths to be passed, etc.) I'd be happy to take a crack at it, if it's a desirable change.

@dsymonds
Copy link
Contributor

GO15VENDOREXPERIMENT support is not going to happen because that env var is going away. It was a transitional hack.

golint isn't going to intentionally skip vendor dirs for the same reason that the go tool doesn't. There' easy workarounds as are listed above.

The only remaining piece is to make sure that vendored imports are actually type checked correctly, and that only requires switching golint to using the go/types package in the standard library. I'll be doing that shortly.

@dsymonds dsymonds self-assigned this Mar 17, 2016
@nathany
Copy link

nathany commented Sep 27, 2016

It would be very nice if golint $(go list ./... | grep -v vendor) worked for consistency between go tools -- such as when creating a Makefile. As mentioned in #151 (comment), golint doesn't like a list of packages.

open github.com/org/repo: no such file or directory
open github.com/org/repo/pkg: no such file or directory

@toxeus
Copy link
Author

toxeus commented Aug 25, 2017

@dsymonds wrote:

golint isn't going to intentionally skip vendor dirs for the same reason that the go tool doesn't. There' easy workarounds as are listed above.

With the release of Go 1.9 the go tool is intentionally skipping vendor dirs. See the release notes and the discussion on github. I suggest that for the same reason golint should also intentionally skip vendor dirs.

@nathany
Copy link

nathany commented Aug 29, 2017

see #320 as this issue is closed

@toxeus
Copy link
Author

toxeus commented Aug 29, 2017

@nathany Github issues are like doors - it's possible to re-open them when they're closed 😉

The reason why I've commented in this issue is because @dsymonds has explained in this issue that one goal for golint is aligning its CLI with go tool's CLI.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

8 participants