rustc-serialize
rustc-serialize copied to clipboard
rust-lang-deprecated
"unimplemented!" encode/decode implementations for wasm
The cfg() guards around the encode/decode implementations result in the crate being unusable on wasm32-unknown-unknown.
These lines don't implement the functionality for that platform (there is file system equivalent there), which makes the crate usable again for file system unrelated tasks like encoding using the "cbor" crate.
This implementation is wrong. If you use the emscripten target, this now compiles in both the unix implementation and the unknown implementation.
Also, can we get this merged? At the moment the num crate doesn't compile because of rustc-serialize not compiling. Not sure why they still use it, but I guess they can't switch to serde before the next major version.
I was unaware that emscripten sets the "unix"-cfg; is there any way to have an "else" part in the cfg variations to do this cleaner, or should I rephrase the non-implementation to be #[cfg(not(any(target_os="redox", unix, windows)))] guarded?
You can't do else afaik, so you could either do the not(any(...)) or specify wasm32, not emscripten. I think the former makes more sense, as it automatically supports all other targets too
We'd like this merged too, and rand version bumped 0.4, which had a wasm fix in https://github.com/rust-lang-nursery/rand/pull/197.
Updated the commit to reflect CryZe's findings about wasm-emscripten; tested under unix and wasm32-unknown-unknown.
Rust recently added CloudABI support, and this will cause encode/decode to become unimplemented! for target_os = "cloudabi", which I don't think is correct.
The CI fails to compile this because the latest rand doesn't compile on 1.0 anymore. So this PR itself compiles just fine. It'd be nice if someone could tell us what we should do about this (maybe just remove the 1.0 compilation?) or just merge the PR regardless.
@kennytm well that didn't compile before at all then. But I guess we can fix it in this PR too.
Instead of removing 1.0.0, better just restrict the rand dependency to a version which 1.0.0 can work, something like:
[dev-dependencies]
rand = ">=0.3.0, <=0.3.20"
(Note: I haven't checked which rand version does work on 1.0.0. The last successful build uses 0.3.16)
May I ask that we keep unrelated issues (like rand versions) in their own issue?
I'm happy to address regressions this issue introduces, but target_os="cloudabi" looks like something that wouldn't work before this pull request (unless windows or unix are set); so if cloudabi yet another implementation of Encodable and Decodable, I suggest that a version be branched off this PR that adds the implementations and adds ,target_os="cloudabi" to the not-any clause to avoid merge conflicts.
@alexcrichton is there a way to get this PR moving again? From what I understand it seems critical to getting a lot of Rust projects that still depend on rustc-serialize into wasm. Obviously the best solution would be to switch projects to serde, but this seems like a good stopgap for cases where that's unfeasible.
This library is currently unmaintained and not publishing new releases, so there is currently not process or timeline for publishing this. Crates depending on rustc-serialize which also want to support new platforms will need to update to a serialization framework like Serde
@alexcrichton There's still a lot of ecosystem critical libraries like num that still depend on rustc-serialize for semver reasons. So I think a new release for this makes sense.
If there's no maintainer here willing to merge and publish this, would it make sense to give someone like tomaka the rights to just push this real quick? This doesn't seem like it would take a lot of effort and affects the new wasm target a lot.
Is it possible to get this going? Many community packages are depending on this. rust-crypto for example.
This library is currently unmaintained and not publishing new releases
The README states:
No new feature development will happen in this crate, although bug fixes proposed through PRs will still be merged.
This PR looks to me like a bug fix and one that really affects the experience of targeting WASM from Rust.
Crates depending on rustc-serialize which also want to support new platforms will need to update to a serialization framework like Serde
This is an option for crate maintainers, but not users of those crates (which may no longer be maintained).
Why don't we make the implementation that's currently used for redox work for not(any(unix,windows))? That should work for most platforms.
