Comments (13)
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?
from webwire-go.
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.
from webwire-go.
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
from webwire-go.
As of commit #216d118 support for dep
is added.
Gopkg.toml
and 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 get
able. 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.
from webwire-go.
I haven't seen an enforcement of the vendor
folder before. Is this project not go get
able?
from webwire-go.
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?
from webwire-go.
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, andgo get -u
will work. - Add
Gopkg.toml
andGopkg.lock
for dep (and user) to know what version is used.
The current approach, in my opinion, is the worst of two worlds.
from webwire-go.
inserting dependencies as actual copies and not submodules or similar link based solutions
Oh no, I'm not proposing submodules or symlinks. Please no.
from webwire-go.
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.
from webwire-go.
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.
from webwire-go.
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.
from webwire-go.
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. 😄
from webwire-go.
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.
from webwire-go.
Related Issues (20)
- Duplex Request-Reply HOT 2
- Request delivery order HOT 2
- Deferred client-side session destruction HOT 1
- Performance Benchmarking HOT 3
- Unsynchronized session getter HOT 1
- Use *log.Logger instead of io.Writer for logging HOT 1
- Listing all connections of a session HOT 1
- Deferred client agent closure HOT 1
- Request message requires a payload HOT 2
- Support for concurrent message processing HOT 1
- Update session relevance HOT 1
- Limit concurrency per client HOT 2
- Support for JSON message encoding HOT 1
- Add support for context.Context HOT 1
- Support multiple underlying websocket connections HOT 2
- Test hot server-side handling of messages violating the protocol
- Test the default session manager implementation.
- Add new constructor function with TLS support HOT 1
- TLS handshake error doesn't stop autoconnect HOT 1
- Subprotocol Support
Recommend Projects
-
React
A declarative, efficient, and flexible JavaScript library for building user interfaces.
-
Vue.js
🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.
-
Typescript
TypeScript is a superset of JavaScript that compiles to clean JavaScript output.
-
TensorFlow
An Open Source Machine Learning Framework for Everyone
-
Django
The Web framework for perfectionists with deadlines.
-
Laravel
A PHP framework for web artisans
-
D3
Bring data to life with SVG, Canvas and HTML. 📊📈🎉
-
Recommend Topics
-
javascript
JavaScript (JS) is a lightweight interpreted programming language with first-class functions.
-
web
Some thing interesting about web. New door for the world.
-
server
A server is a program made to process requests and deliver data to clients.
-
Machine learning
Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.
-
Visualization
Some thing interesting about visualization, use data art
-
Game
Some thing interesting about game, make everyone happy.
Recommend Org
-
Facebook
We are working to build community through open source technology. NB: members must have two-factor auth.
-
Microsoft
Open source projects and samples from Microsoft.
-
Google
Google ❤️ Open Source for everyone.
-
Alibaba
Alibaba Open Source for everyone
-
D3
Data-Driven Documents codes.
-
Tencent
China tencent open source team.
from webwire-go.