cmd/go: exclude vendor dir from matching `...`

[natefinch]

There are some closed issues about this, but I'd like to bring it up again.

Any time you're building an application and you make an update to any package in the repo (of which there are often many), you run go test ./... to make sure you haven't broken your program. This worked fine before vendoring became a thing, but with vendoring, it runs all the tests of vendored code as well. While you may want to do that once in a while, such as when you update the code in the vendor directory, it's generally not something you want to do every time your code changes (because it shouldn't affect code you depend on at all).

I'd like to suggest that ... ignore the vendor directory by default. I almost never want to have any command touching the vendor directory directly - not go vet, not go fmt, not go build, not go test, not go install, not go fix.

If the community feels that there is utility in being able to match the vendor directory, I suggest adding a flag called -vendor (or whatever) that indicates ... should match the vendor directory. But by default, I think almost everyone would prefer it not match the vendor directory.

As we start down the path of having an official package management solution, I think vendoring will get more and more common.... and this will become more and more of a pain.

(and yes, I know you can do the go test $(go list | grep -v vendor) thing, but that's shell and OS-dependent, and ugly, and just bad UX)

[mvdan]

I feel similarly about this issue.

vendor could be ignored when walking its parent, so ./... would ignore it but ./vendor/... wouldn't.

[dlsniper]

I would like to add the votes of 21 people that do not want to have their editor run go test against vendor/ at all https://youtrack.jetbrains.com/issue/GO-2366 and 19 others that don't want gofmt to run against vendor/ as well https://youtrack.jetbrains.com/issue/GO-2412. One could argue that the editor should be smarter and send a list of packages to the tool but then why should users be forced to either have to use $(go list | grep -v vendor), which mind you is not possible on Windows, or use an editor to achieve this? WSL does not count as it introduces yet again more work for the user, thus bad UX.

Also, please notice tools like glide, one popular dependency management tool, where there's even a dedicated command to list packages and omit the vendor directory in order to make life easier.

This is clearly an UX problem that the users should not have to face.

[ianlancetaylor]

Previous discussion:

• 11659 exclude vendor directories from wildcard matches?

• 14417 cmd/go: go test ./... fails on go 1.6 because dependencies are tested

• 17497 cmd/go: go generate ./... will recurse into vendor

• 17879 Exclude vendor/ from linters

Marking as Go 1.9 for further consideration and decision.

[dlsniper]

I would also like to add that this is what a Windows user currently has to write when using Windows CMD to work around the vendor issue

for /f "" %G in ('go list ./... ^| find /i /v "/vendor/"') do @go test %G

which is a lot worse than go test $(go list | grep -v vendor).

[ernesto-jimenez]

@natefinch thanks for raising this again.

Regarding the usual argument that "./... means all sub packages should avoid making vendor a especial case":

1. vendor is an especial case.
2. ./... already ignores folders with names starting with . or _, so there's a precedent supporting this proposal.

[PaulReiber]

Are there good reasons why the vendor directory cannot simply be renamed _vendor?

Kind regards,

Paul Reiberhttp://about.me/reiber

On Tue, Feb 14, 2017 at 2:53 PM, Ernesto Jiménez notifications@github.com wrote:

@natefinch https://github.com/natefinch thanks for raising this again.

Regarding the usual argument that "./... means all sub packages should avoid making vendor a especial case":

1. vendor is an especial case.
2. ./... already ignores folders with names starting with . or _, so there's a precedent supporting this proposal.

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/golang/go/issues/19090#issuecomment-279862554, or mute the thread https://github.com/notifications/unsubscribe-auth/AADlQB4xORqJ5Z9Y1dbkOTD-C4dlEPLxks5rcjBWgaJpZM4MBALL .

[dlsniper]

@PaulReiber see this comment: https://github.com/golang/go/issues/11659#issuecomment-173376808

"vendor" is not being renamed. That ship has long sailed.

@ianlancetaylor can you please add #11659 to the list of related issues as hopefully your comment will be more visible than mine. Thank you

[ianlancetaylor]

@dlsniper Done, thanks.

[rakyll]

This was overwhelmingly mentioned in the tools survey I conducted last year. Combining all feedback I got (individuals, mailing list questions, StackOverflow, etc), this particular problem was one of the top problems that has been mentioned.

[ghost]

A core function like test that would understand vendoring as a dependency and not the target of testing scope would be huge. It's inefficient to write custom regex patterns to include some portions of code and not others. Increasingly so when you want to operate on subsets of the code base. The ./... Operator forces you to glob all go files with the expressed understanding that you will filter out a majority of the results returned wastes cycles during iterative testing. Given this is a solved problem with tools like glide it would seem like a quick win scenario with tangible returns to the go dev community.

[minux]

This has be discussed at length and rejected.

I think the real solution is #11193. What people really don't want is to run unneeded tests.

With proper test caching, it doesn't matter go test ./... tests vendored packages or not.

[natefinch]

That doesn't help for all the rest of the tooling. I don't want go vet complaining about problems in vendored code. I definitely do not want go fmt or go fix changing things in vendored code. I don't want go install installing binaries that happen to be in vendored repos.

The root of it is this: without vendoring, ... never matches dependencies that arent't in your repo, because they're not subdirectories. I just want to maintain that functionality while vendoring.

[minux]

Then don't invoke go fmt or go fix on ./..., which is very problematic even if there aren't any vendored packages.

The core problem is people tend to overuse the ./... pattern. The vendor problem is just a red herring.

[dlsniper]

I've also just got bitten by an issue that go vet complains about code in the vendor/ directory but until the upstream changes that code I can't do anything there thus I'm forced to use the workaround instead of not having to deal with it at all.

And I don't think that go fmt ./... is overused when you want to format the project and make sure it's all consistent inside a pipeline for example.

[minux]

My proposal (again) is to introduce negative patterns explicitly.

e.g. go test ./... -./vendor/... # using - is preferred, but -exclude also works.

to exclude any packages matching ./vendor/... from the result of ./....

Changing the behavior of ./... today to ignore vendor packages will only introduce confusion. (And there are reasonable uses of ./... to match vendor packages.)

[sparrc]

Then don't invoke go fmt or go fix on ./..., which is very problematic even if there aren't any vendored packages.

why is this "problematic"? We've never had any problems with it, and we run it on every commit merged into InfluxDB, Telegraf, & Kapacitor.

and I personally don't care if my dependencies are go fmt'ed properly, I only want to run it on my codebase.

Changing the behavior of ./... today to ignore vendor packages will only introduce confusion.

My experience has been most people being confused by it not ignoring vendor/.

[mibk]

e.g. go test ./... -./vendor/... # using - is preferred, but -exclude also works.

While this concept could definitely be applied to more use cases, excluding vendor is the real pain IMO. For other use cases go test $(go list | grep -v ...) always does the job.

Changing the behavior of ./... today to ignore vendor packages will only introduce confusion. (And there are reasonable uses of ./... to match vendor packages.)

If we don't want to break backward compatibility and therefore prevent potential confusion, could it be tolerable by both sides to introduce a single-character flag for excluding vendor? It would be only 3 keystrokes more to type without any BC break.

[hodgesds]

What about a .gorc file that can be configured with defaults? That way it can be configured globally ( ~/.gorc ) if you hate doing grep -v or set within a package when running tool on vendor/ is desired.

[minux]

configuration files are out of the question. the go command is designed to avoid configuration files in the first place.

Also, whether to ignore a package is property of a specific invocation of the go command.

Please note that we do want to build/install the vendored packages, so only ignoring them in test, vet, fmt, but not build looks pretty inconsistent to me. That's why I propose adding negative patterns to make it explicit.

To put it another way:

should the ./... pattern matching different set of packages in the following commands? go install ./... go test ./...

[mvdan]

@minux why would you want to explicitly install vendored packages? If they are vendored, they are dependencies and will already be built if vendor is ignored.

[sparrc]

Please note that we do want to build/install the vendored packages

@minux, that's not true, in telegraf I only want to build the packages in vendor/ that telegraf depends on, not every single package in vendor/.

Building every package in vendor/ means that you would have to trim out packages/tests that your project doesn't use.

This is not how it works when your dependencies are in your GOPATH.

[minux]

Whether to include unnecessary packages under vendor depends on how you vendor a package (e.g. copy or git submodule), and that's not relevant in this discussion.

If you vendor only necessary packages, then you could use go install ./....

[mvdan]

If you vendor only necessary packages, then you could use go install ./....

True, but it still doesn't explain why you need the vendored packages to be included. This just explains a way for it to not matter.

[davecheney]

Rather than expressing incredulity that people want to use the glob operator, I think it is important to recognise that globbing is a straight forward solution to very commonplace questions like "How can i run the tests for all the packages in my project?", which, until go 1.6, was satisfied by globbing.

Saying that people shouldn't use the glob operator is not helpful as the replacement was non portable (how do you pipe grep on windows?), or relies on an external tool like go test $(glide nv); which also isn't a portable solution.

As @rakyll showed from her research this issue is a major point of confusion for Go developers and that alone should be cause to reexamine the decision.

[iand]

Another consideration is that the current behaviour requires you to vendor dependencies of your dependency's tests, e.g. packages like testify

[jsternberg]

Related to the above consideration, go get ./... will not bring in those dependencies automatically so you have to go get ./..., vendor the dependencies, then repeat until no new vendored dependencies show up. While it's easy enough to make a custom tool that does this, it seems like it should be unnecessary for the default go toolchain to cause this problem.

It seems to me like you should be able to call go get ./..., vendor the dependencies identifies and retrieved by the command, and go test ./... would still work. This isn't currently true.

[davecheney]

@jsternberg if possible I'd like to leave go get ./... out of this debate. I want everyone to remain laser focused on the usability issue of go test ./... matching vendored code.

There are certainly other issues with the Go 1.6+ behaviour, but for the sake of clarity, can those debates be taken elsewhere.

Thanks

[natefinch]

@davecheney I actually don't want to limit this just to go test. While I agree that go get is a special flower, almost none of the normal go tools would seem to be appropriate to run on the vendor directory.

[slrz]

@natefinch Why not? The list command comes to mind immediately.

@davecheney How can they be separated? Are you suggesting the possibility of making the glob work differently between test and get? Ugh. Is there any precedent for this in the existing code?

[davecheney]

I just want people to focus on the problem of go test in this issue.

There are other use cases, they should be argued separately as in my experience if you ask for several things at once, you risk getting none of them.

On 17 Feb 2017, at 06:59, slrz notifications@github.com wrote:

@natefinch Why not? The list command comes to mind immediately.

@davecheney How can they be separated? Are you suggesting the possibility of making the glob work differently between test and get? Ugh. Is there any precedent for this in the existing code?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or mute the thread.

[StabbyCutyou]

I would agree that focusing this only on the UX issues faced by go test is the best move for this proposal.

That said, longer term it would be great if the behavior change becomes standard in other places where the ./... syntax is supported.

But i'd like to see us fix the leaky pipe before trying to refinish the basement. I run into this all the time in many different projects and the lack of a simple solution built into the test tool itself has long been a pain point for myself and many others I know.

[nathany]

This is a concern across several tools, not just go test ./.... A holistic solution would be appreciated, even if this serves as a meta-issue with separate issues specific to each tool.

As mentioned previously in @rakyll's survey, my main issue is that there isn't a consistent workaround for ignoring the ./vendor folder with the variety of Go tools.

The gofmt tool requires a list of files, it doesn't work with a $(go list ./... | grep -v vendor) exclusion.

The result is a Makefile with a bunch of different commands instead of something simple.

GO_PACKAGES = $(shell go list ./... | grep -v vendor)
GO_FILES = $(shell find . -name "*.go" | grep -v vendor | uniq)

gofmt -s -l -w $(GO_FILES)
go vet $(GO_PACKAGES)
go list ./... | grep -v /vendor/ | xargs -L1 golint
go test $(GO_PACKAGES)

It took a good amount of time to find the quirks and come up with commands (we're not all bash experts). Multiply that by every team out there running into this. As pointed out in this thread, it gets even worse, in that these commands are not cross-platform.

Specifying ./... and perhaps a -novendor flag across any tool would be simple.

If there is a flag, what it's named, and whether the default is to include vendor or not, I don't have an opinion right now. What's important to me is that the same pattern works across tools.

[robbiev]

I'm not sure if this has been mentioned already: what about supporting a _vendor directory in addition to vendor?

[StabbyCutyou]

If we could get the issue addressed in test first, get the behavior correct and make sure it's working as intended, then i'd imagine it would be easier and more palatable to propose rolling the changes to the vet tool and others.

If we get it fixed just in test or globally, however, i'll be happy either way.

[cznic]

First, we've got another special treatment of (yet another) special name the go tool has to handle differently.

Secondly, we saw it has unforeseen consequences.

Thirdly, we propose to fix that by adding another specially treated case.

Featuritis

[davecheney]

Thirdly, we propose to fix that by adding another specially treated case.

I argued at the time, and will do so again that the vendor directory should have been called _vendor/. This would have had less impact on tooling

[dlsniper]

I don't understand why we can't consider this as a quality of life improvement based on concrete facts and evidence. It's clear that people don't want tools that use ./... to touch vendor/ at all most of the time.

The question should be: Does it have to be implicit or explicit to omit it? Not if we should do it or it. There's clear evidence that some users have a lot harder time around this missing functionality just by looking at the Windows users and the horrible workaround they have to do.

More evidence this is needed? Hang out long enough in Gophers Slack, read the various mailing lists / issue trackers of various tools or do a Google Search for "go test" exclude vendor to find the 10k+ results around this (can't validate them all but they are there if someone wants to tackle this).

My gut feeling is that this option should be explicit in usage so that the current behavior is not changed for the tools we have and be an opt-in feature, as it should have been from the beginning to include the vendor.

[ScottBrooks]

I feel like the issue can be better understood as

"After adding vendor support, there is no easy/cross platform way to test just my code"

./... is not intuitive either, it just happened to be what worked.

Adding the following option to go test would avoid needing to change how ./... works, and would potentially work cross platform.

     -l
         Run the tests on your local files only, excluding the vendor directory.

[franciscocpg]

I agree with @dlsniper and @ScottBrooks. It's better (less confusing) to add a new flag and keep backwards compatibility than changing current default behavior.

[sparrc]

just want to re-iterate that this should not only apply to go test. IMO it would be even more confusing if one of the go tools behaved differently than the others, and I don't think the anybody wants to fmt their vendored dependencies either.

[spenczar]

I am confused by the arguments against this which object on the grounds that it is "yet another special case." vendor is a special directory. It's just incompletely implemented as such today. Some tools treat it as special, others do not.

I think this is a question of consistency in understanding that vendor is a special directory. That's why people get confused and surprised - they expect the special-ness to be consistently handled.

So: this isn't "yet another" special case. This is the same old one, just completing its implementation.

[dlsniper]

@ianlancetaylor / @rsc et.al. involved in the dev team, could you please help / guide us on this one.

Personally, I'd love to see this landing in time for 1.9 if possible, and judging by the number of voters (according to NoMeToo this should be the way to show support for it), I believe that others would be interested in this sooner rather than later.

Thank you for your time and consideration.

[dlsniper]

P.S. it doesn't mean the Go dev team has to work on the implementation by all means, just accept / decline it, I'm sure the implementation will follow.

[smo921]

Some thoughts from a Go user who finally needed vendoring, and probably has no business even responding to this discussion. To preface this: I love the Go community and the language. I encourage people to try it out. I've even presented an intro to Go to my team in hopes we would start writing more Go code. My background is systems operations, not development. I have however dealt with dependency hell.

The TL;DR: Basically what @spenczar said above.

Now, on to my $0.02.

1. The choice of vendor as the directory name means I can no longer have a package named vendor. It essentially introduced a new reserved word into the language. I don't foresee needing a package named vendor in my software, but someone might.

2. The first time using vendor/ and running go test ./... I was met with a slew of errors from my dependencies. This led to frantic searching for workarounds and advice from the community. To be 100% honest, I don't care about running vendor tests, they are not code I own, nor are they tests I want to run. Perhaps I'm naive, I don't write software all day long and generally trust library authors to properly test their code before it is released.

3. Whatever the fix (there are plenty of good ideas), don't let a desire for 100% backward compatibility influence the decision. You've already introduced breaking functionality by using a non-reserved name (vendor), instead of something that the golang toolset would ignore by default (_vendor or .vendor for example)

4. Trying to encourage people to test drive Go without dependency management (ie vendoring) was difficult. Now that vendoring has been officially adopted the reluctance to make "non-backward compatible" changes to the tool set is frustrating. It is one more strange edge-case that needs to be explained to newcomers.

My ideal solution would be to have the location of vendor dependencies be ignored by the golang toolset by default. The subtree should be treated as if it didn't exist or was read-only. I shouldn't fear that running go fmt is going to modify the source code of my dependencies. Making the default behavior ignore the vendor directory also means that existing tools / editor plugins / etc don't need to be updated to deal with this new special directory.

In the mean time I've done the symlink trick, and renamed vendor to .vendor and then created a link between the two.

Thanks.

[ianlancetaylor]

@dlsniper The golang/dep project is overhauling dependency management, I think when that process is completed we will have a better understanding of how to handle ... and the vendor directory.

[davecheney]

Respectfully, dep is an experiment and focused on an entirely different problem; versioning Go packages.

This issue is about ... matching vendor/ and the effect is has on developers edit/compile/test inner loop.

I would appreciate it if every problem with the Go tool was not kicked down the road by saying "oh dep'll fix it"

Thanks

Dave

On 28 Mar 2017, at 10:31, Ian Lance Taylor notifications@github.com wrote:

@dlsniper The golang/dep project is overhauling dependency management, I think when that process is completed we will have a better understanding of how to handle ... and the vendor directory.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or mute the thread.

[dlsniper]

@ianlancetaylor As per comment here: https://groups.google.com/d/msg/golang-nuts/PaGu2s9knao/Bq4vmFh7AgAJ it seems that golang/dep will not arrive here in 16 months from now as an official tool, and I'm not sure about others, but I'd rather fork the Go frontend and add that option rather than wait that long.

There are clear signs everyone really do not agree with the current situation and we, the community, want to work with you, the Go team, to get this improved for us, all the Go users.

If creating the CL for this issue is what stops us from having a decision on it, then please indicate so, but given how this is a proposal, even if it doesn't follow the normal proposal flow, I think it must be addressed. Go 1.9 cut-off will be here in almost a month from now, at which point it will be too late for us to change anything for the next few months, which can potentially delay it to Go 1.10, that means, 10 more months to get a --no-vendor or --skip-vendor or --omit-vendor flag.

Thank you for your time and consideration.

[rsc]

I wrote in https://github.com/golang/go/issues/11659#issuecomment-171665255:

I think there are two cases:

1. A package in a vendor tree that is actually used (directly or indirectly) by the code outside the vendor tree. In this case I think you would want to test the package, just as you test all your other dependencies. If code you depend on is broken, you want to find out in the most precise test possible, not wait until a test of the main program fails mysteriously.
2. A package in a vendor tree that is not actually used (directly or indirectly) by the code outside the vendor tree. In this case, obviously no one cares and the test need not run. But in this case I don't understand why tools are bringing that package into the vendor tree in the first place. I would argue that, for many reasons, dead (unused) code should not be copied into a vendor tree. Code that is in your vendor tree has to be maintained, updated, and so on. If it's not needed, it should just be omitted.

I think it's a mistake to skip the test in (1). And I think (2) indicates a problem in the vendoring process.

The counter-argument to this is that you can't omit individual packages when you're using git submodules. But when you're using git submodules I think you're tying yourself to the target repository quite a bit more and so it may still be appropriate to test the entire thing.

Obviously from the reactions to the issue lots of people are still running into problems. But why?

Is it because there is dead code in your vendor trees you want to ignore?

Is it because you don't want to test the vendored code you do use?

[bradleyfalzon]

Is it because you don't want to test the vendored code you do use?

Because you asked: I don't run the tests from the standard library, I don't see why I should run the tests for vendored code either. I want to quickly run my tests, so I can continue developing.

[cespare]

At our company we import vendored code without test files, so go test is not an issue. But we have to explicitly ignore vendoring whenever we run go vet, golint, or other static analysis tools. We even have some vendored code that isn't gofmted, sadly, so running a project-wide gofmt also requires ignoring vendor.