cmd/go: exclude vendor dir from matching ...
#19090
Comments
|
I feel similarly about this issue.
|
|
I would like to add the votes of 21 people that do not want to have their editor run Also, please notice tools like This is clearly an UX problem that the users should not have to face. |
ianlancetaylor
changed the title
......
|
Previous discussion:
Marking as Go 1.9 for further consideration and decision. |
|
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 which is a lot worse than |
|
@natefinch thanks for raising this again. Regarding the usual argument that "
|
|
Are there good reasons why the vendor directory cannot simply be renamed
_vendor?
Kind regards,
*Paul Reiber*http://about.me/reiber
…On Tue, Feb 14, 2017 at 2:53 PM, Ernesto Jiménez ***@***.***> 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
<#19090 (comment)>, or mute
the thread
<https://github.com/notifications/unsubscribe-auth/AADlQB4xORqJ5Z9Y1dbkOTD-C4dlEPLxks5rcjBWgaJpZM4MBALL>
.
|
|
@PaulReiber see this comment: #11659 (comment)
@ianlancetaylor can you please add #11659 to the list of related issues as hopefully your comment will be more visible than mine. Thank you |
|
@dlsniper Done, thanks. |
|
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. |
|
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. |
|
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.
|
|
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. |
|
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.
|
|
I've also just got bitten by an issue that And I don't think that |
|
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.)
|
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.
My experience has been most people being confused by it not ignoring |
While this concept could definitely be applied to more use cases, excluding
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. |
|
What about a |
|
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 ./...
|
|
@minux why would you want to explicitly install vendored packages? If they are vendored, they are dependencies and will already be built if |
@minux, that's not true, in telegraf I only want to build the packages in Building every package in This is not how it works when your dependencies are in your GOPATH. |
|
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 ./....
|
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. |
|
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. |
|
Another consideration is that the current behaviour requires you to vendor dependencies of your dependency's tests, e.g. packages like testify |
|
Related to the above consideration, It seems to me like you should be able to call |
|
@jsternberg if possible I'd like to leave There are certainly other issues with the Go 1.6+ behaviour, but for the sake of clarity, can those debates be taken elsewhere. Thanks |
But it does have to work on Windows, which are the largest population of Go users, and have neither shell aliases or nice things like grep, or substitution. |
|
This hasn't been true for over a decade. Any supported Windows variant includes a shell that, while somewhat different from Unix shells, is very well capable of such things. I wouldn't be surprised if it even provides a |
|
@slrz So with your suggestion windows users will now have to install a shell they're not familiar with, learn that shell, rewrite their scripts in it, and this is easier than you running |
|
@slrz if you could find something simpler than: I'd be happy to hear about it. Meanwhile, this feature was requested by the Go users, which proved with a fairly good technical evidence that not matching I believe that the current way that's implemented offers a good balance of that while still allowing It will rarely be so that you need to type: Most likely you'd need to type: Also, special cases do exist today, afaik So please, try to understand that there are some of us who are really happy to see this, and per the commend here: #19090 (comment) that number might actually be bigger than you think. So a quality of life improvement for so many people while still allowing for the current behavior to exist is a welcomed change imho. I hope this helps seeing "the other side" as well. |
|
@sgen Heck, we're talking about programmers here. They will figure out something to exclude items containing a constant substring from a list. Assuming they even want that particular quirky behaviour instead of the one you'd expect from reading the description of patterns in the help text, without the part about this special case. |
|
@slrz this isn't the quirk. The quirk is that We were in a bad state previously when If purity doesn't convince you, practicality should: In my experience teaching Go and being a local expert in a large-ish company on Go tools, this was an almost universal stumbling block when people went to lint or test their code. Nobody knew to use |
Can you please cite some source of Windows Go users being "the largest population of Go users"? Meanwhile, let me refer to a survey of 3,595 respondents: |
|
The windows version is the most popular download of windows/dl. This was reported to me in private correspondence a year or so ago, I don't have access to verify this fact directly. |
|
Out of respect for everyone, I think we should stop the bike shedding now and maybe open up an issue if we find a problem in the current implementation as asked by @rsc. Finally, please note that comments like The fact that Lets not have the team lock down the discussion as I consider that to be a bit unfriendly. Thank you. |
|
But I want to paint it chartreuse! |
The following should work in a default Windows system, having installed only the Go MSI package:
The |
By overwhelming popular demand, exclude vendored packages from ... matches,
by making ... never match the "vendor" element above a vendored package.
go help packages now reads:
An import path is a pattern if it includes one or more "..." wildcards,
each of which can match any string, including the empty string and
strings containing slashes. Such a pattern expands to all package
directories found in the GOPATH trees with names matching the
patterns.
To make common patterns more convenient, there are two special cases.
First, /... at the end of the pattern can match an empty string,
so that net/... matches both net and packages in its subdirectories, like net/http.
Second, any slash-separted pattern element containing a wildcard never
participates in a match of the "vendor" element in the path of a vendored
package, so that ./... does not match packages in subdirectories of
./vendor or ./mycode/vendor, but ./vendor/... and ./mycode/vendor/... do.
Note, however, that a directory named vendor that itself contains code
is not a vendored package: cmd/vendor would be a command named vendor,
and the pattern cmd/... matches it.
Fixes golang#19090.
Change-Id: I985bf9571100da316c19fbfd19bb1e534a3c9e5f
Reviewed-on: https://go-review.googlesource.com/38745
Reviewed-by: Alan Donovan <adonovan@google.com>
|
My soln till this mess gets sorted is with npm package.json. Something like the following: To test on windows To test on linux/osx |
|
better late than never! |
We update all the vendored dependencies and the golang version
by running:
go get golang.org/x/sys/unix # Explicit because not added by mac
godep get $(GO_PACKAGES)
godep update -goversion
godep update ./...
godep save
We add the resulting vendor directory to the git repo, so we do not
need call godep anymore when calling the go commands[1]. In the
Makefile we add logic to ignore the ./vendor directory as
suggested in[2]
[1] https://golang.org/cmd/go/#hdr-Vendor_Directories
[2] golang/go#19090 (comment)
and others
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)The text was updated successfully, but these errors were encountered: