search

kyren / webrtc-unreliable

Just enough hacks to get unreliable unordered WebRTC data channels between a browser and a server
Other
394 stars 29 forks source link

Discrepency between crates.io, master, and examples. #12

Closed robertwayne closed 4 years ago

robertwayne commented 4 years ago

I'm not sure if this is intentional, but if you pull from crates.io webrtc-unreliable = "0.4", it gets a version that is incompatible with the current examples. In addition, the documentation is not consistent with the current example, though it does match the crates.io version.

If it's just not ready to be pushed to crates.io, perhaps a note in the readme that says if you want to use the examples, you should pull in from the repo instead?

ie. 

[dependencies.webrtc-unreliable]
git = "https://github.com/kyren/webrtc-unreliable"

For what it's worth, and for anyone having issues in the future, I was running into message not being a field on MessageResult struct, which was changed in this commit 2baca95c5e911e80b00f54c969dbced22f0de329. Prior, returned a message_len instead, and the examples would not compile (unless I cloned the repo).

kyren commented 4 years ago

This is a good heads up for anybody running into this issue, but I'm pretty sure this is a problem universal to rust crates on crates.io with public repositories. The examples in the master branch will always be... for the master branch which may or may not be compatible with the one on crates.io. FWIW, I do tag releases of my crates, so you can look under the github releases page to quickly get to the proper tagged crates.io version, or you can get to the same thing via docs.rs I believe.

robertwayne commented 4 years ago

That does make sense when I think about it, hah. Something I will definitely keep in mind in the future.

Thank you!

withtypes commented 4 years ago

@kyren do you need help on the project? What is the current status, I see there are unreleased changes.

kyren commented 4 years ago

@jocaml I'm going to make a release soon because of tokio 0.3, I'm going to make the library runtime independent, write a tokio impl crate, then do a release (I think).

kyren commented 4 years ago

I'm going to make the library runtime independent, write a tokio impl crate, then do a release (I think).

So after thinking about it, I see two main options for runtime independence: async-trait or async-io.

Ideally, all we need for complete runtime independence is an abstract async-ified UdpSocket and Timer traits. An async Timer trait is definitely doable (see the turbulence crate), but the UdpSocket is harder without at least GATs or full async in traits. We could fix this with the async-trait crate at the cost of an allocation per call, or just do the same thing manually by having the trait methods return boxed futures. Probably this is not realistically so terrible, given all the other stuff going on, but I'm not sure.

The other option is simpler, which is just to use async-io and be done with it. This crate is more "polite" than tokio because it starts a reactor thread automatically (rather than panicking) and does not include a "full" runtime including an executor, so in theory using it and its reactor is "transparent" except this of course isn't entirely true, since async-io achieves this transparency by starting a built-in reactor thread that the user doesn't have complete control over.

Most of all right now I just want to make webrtc-unreliable not rely on the user using a specific runtime. I considered a bunch of other weird possibilities like dropping all the way down to sjepang's polling crate and doing everything myself, but this would require creating a thread to drive a reactor JUST for webrtc-unreliable, so that seems probably worse than just using async-io. I also considered writing webrtc-unreliable so that the server was inside out, letting the timing and udp i/o be outside the server and pushed inside, but besides being a lot of work for something maybe kind of stupid, it would probably be extremely hard to use.

Eventually, I think maybe the most correct thing to do is parameterize the server on a Runtime trait that provides UDP + timing, but that's a lot of work for a still sub-optimal outcome right now. Probably this will have an easier answer down the line with GATs and async in traits and some common ecosystem traits and maybe a good resolution to https://github.com/tokio-rs/tokio/issues/2435 so I'm just not going to worry too much about this right now.

So all that being said, for now I think I'm just going to use async-io and be done with it. The single global reactor thread is probably very unlikely to actually matter for most people, but if it does end up mattering please complain to me and I can move faster towards a trait interface, even if it has to use Box<dyn Future>. In the meantime, people should be able to use this crate from any runtime at all or no runtime and it should at least work.

kyren commented 4 years ago

Okay 0.5 is now released! This issue really is unrelated to any particular release, and is not really fixable, so I'm going to close this issue. This will always be an issue with rust crates hosted on github or any public git repository, master branch may just not be compatible with the version on crates.io :woman_shrugging: