Consider using dep for vendoring

[AlekSi]

That will simplify life for those who use both this library and gorilla/websocket directly.

[romshark]

I'm not sure if this is the right approach since the current solution (inserting dependencies as actual copies and not submodules or similar link based solutions) offers very appealing guarantees:

• The library doesn't depend on remote sources and doesn't therefore require any additional setup steps.
• The built is guaranteed to remain consistent as there's no unpredictable remote factors affecting it.
• Will work in any environment, be it dep, glide, the standard go tool chain or whatever.

If dependencies mutate or disappear for whatever reason then this library won't be affected at all and will continue produce predictable builds. If the vendor directory was big with lots of dependencies then it'd be a problem, but it's only 228KB, not much to be honest, and it could even be stripped down to 160 removing non-source-code stuff such as the examples.

Is there something special about dep that I'm missing here?

[nkev]

I haven't seen an enforcement of the vendor folder before. Is this project not go getable?

[romshark]

go get should work just fine.

As of Go 1.6 the vendoring feature is enabled by default and as we do not plan to support anything below 1.6 all dependencies are embedded into the vendor directory for the reasons described above.

Is there still any problem with the current solution or can I close this issue?

[romshark]

Talking about go get by the way one should always remember that go get clones the HEAD of the master branch which isn't currently considered a stable "release" branch, at least not until v1.0.0 rc1.

[AlekSi]

Currently, you vendor gorilla/websocket by hands, without using any tools. This approach has two issues:

• user can't know what version of gorilla/websocket you use: is it some released version? some random commit from master branch?
• user's dependencies management tool (like dep) can't know what to do if gorillia/websocket is used by the user's app too.

I propose one of the two approaches:

• Remove vendor/. gorilla/websocket has stable API for years now, and go get -u will work.
• Add Gopkg.toml and Gopkg.lock for dep (and user) to know what version is used.

The current approach, in my opinion, is the worst of two worlds.

[AlekSi]

inserting dependencies as actual copies and not submodules or similar link based solutions

Oh no, I'm not proposing submodules or symlinks. Please no.

[romshark]

user can't know what version of gorilla/websocket you use: is it some released version? some random commit from master branch?

We only use stable versions from the official releases, currently it's v1.2.0. If we ever change or update the underlying socket implementation we will mention that in the change log for the evolution to be transparent to our users.

user's dependencies management tool (like dep) can't know what to do if gorillia/websocket is used by the user's app too.

I think neither library users nor their dependency management tools need to know or care that gorilla/websocket is used internally. We're providing a websockets abstraction after all so you don't have to care about low level stuff. Your dependency management tool thus should consider qbeon/webwire-go and gorilla/websocket a single piece of code.

Remove vendor/. gorilla/websocket has stable API for years now, and go get -u will work.

go get will get the latest commit from the master branch, which is totally not safe. I do trust gorilla/websocket though I always prefer to have immutable copies of dependencies embedded in the code if it's not too bulky ofcourse (which it's not in this case, just 200kb) so the build always remains consistent, no matter what.

I'm trying to prevent getting tons of issues referencing strange bugs which we then have to spend time debugging on just to find out It's a problem caused by the user fetching the wrong version of the dependencies. Nobody wants that, so better keep dependencies embedded and immutable.

Oh no, I'm not proposing submodules or symlinks. Please no.

By "link-based approach" I meant when there's no copy of the dependencies embedded and you'll have to explicitly get it from a remote source, be it a submodule, a reference from Gopkg.lock or whatever.

Summary

• Does the current approach break any dependency management tool?
  As far as I know, as of Go 1.6 - it doesn't, or does it?

• Will support for dep break other dependency management tools?
  We don't want to enforce any kind of dependency management technique for this library

• How do we prevent users using different kinds of dependency management techniques from blaming webwire-go when it's actually the fault of a wrong dependency version?
  Nobody wants to spend time investigating those issues.

[AlekSi]

I think neither library users nor their dependency management tools need to know or care that gorilla/websocket is used internally. We're providing a websockets abstraction after all so you don't have to care about low level stuff. Your dependency management tool thus should consider qbeon/webwire-go and gorilla/websocket a single piece of code.

Sorry, but that's not true. gorilla/websocket leaks into webwire's public API: https://godoc.org/github.com/qbeon/webwire-go#NewClientAgent And it can't be used as-is: https://github.com/AlekSi/webwire-vendoring/

cannot use conn (type *"github.com/gorilla/websocket".Conn) as type *"github.com/qbeon/webwire-go/vendor/github.com/gorilla/websocket".Conn in argument to webwire.NewClientAgent

To make it work, both gorilla/websocket and webwire should be vendored, and nested vendor/ directory should be stripped. And since there is not machine-readable information about the version of gorilla/websocket you use, some other version may be used.

See also https://github.com/golang/dep/blob/master/docs/FAQ.md#my-dependers-dont-use-dep-yet-what-should-i-do

[romshark]

Sorry, but that's not true. gorilla/websocket leaks into webwire's public API

It's good that you mention that, this is obviously a mistake and I'll fix it as soon as I can, thank you!
webwire.NewClientAgent shouldn't even have been exported.

Normally users won't confront gorilla/websocket when using qbeon/webwire-go so the fact that webwire is an abstraction hiding the internal websocket implementation for good reason remains true.

The 3 questions from the above summary thus still remain unanswered.

[romshark]

As of commit #2daff67 gorilla/websocket is no longer exported to the public API of qbeon/webwire-go. All socket interactions are now done through the new abstract socket and upgrader interfaces which make the underlying socket implementation interchangeable.

This should make dep work without requiring gorilla/websocket.

[AlekSi]

This should make dep work without requiring gorilla/websocket.

But that assumption should be checked. :) I checked it for you: https://github.com/qbeon/webwire-go As you can see, vendor/ is still stripped, and gorilla/websocket is a top-level dependency. If you really want to make it internal, you should use /internal/ packages: https://golang.org/s/go14internal

But so far it looks to me that you actively fighting against Go best practices for dependencies management. You asked for feedback and review, and I delivered. And now I'm going on vacation. 😄

[romshark]

I checked it for you: https://github.com/qbeon/webwire-go As you can see, vendor/ is still stripped

First, thank you very much for taking your time, we appreciate it!

Does this mean dep automatically removes any vendor directories in the packages it imports?
Does this answer the first question about "breaking any dependency management tools"?

But so far it looks to me that you actively fighting against Go best practices for dependencies management.

I'm not fighting against best practices, actually I'm trying to ensure all of the three questions above ↑ are answered properly to:

• A: avoid binding webwire-go to any specific tools and requirements, ideally it should work in any valid Go environment, even though dep seems to become the defacto standard for dependency management in Go - we can't enforce the use of dep as there's still other tools and techniques out there like glide, gb etc.
• B: ensure we won't have to debug bugs caused by incorrect dependencies fetched by the user, thus we try to embed the single dependency into the package keeping the build consistent no matter what.

[romshark]

As of commit #216d118 support for dep is added.
Gopkg.tomland Gopkg.lock are both present for dep to be able to work properly when the library users uses gorilla/websockets alongside webwire.

The embedded vendor directory containing the default gorilla/websocket depedency won't be removed for this repository to remain go getable. The dep FAQs clearly state there's nothing wrong about having an embedded vendor in the repo.

When go get is used, embedded dependencies are used from the embedded vendor. When dep is used, the internal vendor is stripped and the dependencies move to the actual projects vendor directory.

I consider this issue resolved, if there's still anything to be discussed then we can reopen it any time.