Skip to content

Latest git not building for wasm #194

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
singalen opened this issue Feb 17, 2021 · 21 comments
Closed

Latest git not building for wasm #194

singalen opened this issue Feb 17, 2021 · 21 comments
Assignees
Labels
awaiting_confirmation Believed fix, waiting for testing bug Something isn't working

Comments

@singalen
Copy link
Contributor

I have this in my cargo.lock:

version = "0.8.1"
source = "git+https://github.com/thebracket/bracket-lib.git#2c52c1b96de26dac4d669657937167cf25afc46f"
dependencies = [
...
 "ctrlc",

When I try to build for wasm, I get:

$ cargo build --release --target wasm32-unknown-unknown
   Compiling ctrlc v3.1.7
   Compiling paste v1.0.4
   Compiling downcast-rs v1.2.0
   Compiling ucd-trie v0.1.3
   Compiling unicode-segmentation v1.7.1
   Compiling proc-macro2 v1.0.24
   Compiling syn v1.0.60
   Compiling log v0.4.14
error[E0432]: unresolved import `platform::Signal`
  --> /Users/vsergiienko/.cargo/registry/src/github.com-1ecc6299db9ec823/ctrlc-3.1.7/src/lib.rs:53:9
   |
53 | pub use platform::Signal;
   |         ^^^^^^^^^^^^^^^^ no `Signal` in `platform`

error[E0412]: cannot find type `Error` in module `platform`
  --> /Users/vsergiienko/.cargo/registry/src/github.com-1ecc6299db9ec823/ctrlc-3.1.7/src/error.rs:25:23
   |
25 | impl From<::platform::Error> for Error {
   |                       ^^^^^ not found in `platform`
   |
help: consider importing one of these items
   |
1  | use Error;
   |
1  | use error::fmt::Error;
   |
1  | use std::error::Error;
   |
1  | use std::io::Error;
   |

...and a bunch of other errors.

MacOS,

$ rustc --version
rustc 1.50.0 (cb75ad5db 2021-02-10)
@justinbowes
Copy link
Contributor

justinbowes commented Feb 21, 2021

I have the same issue with 0.8.0 and 0.8.1. If I borrow dependencies from amethyst/rustrogueliketutorial@3792f07 I can get it to work.

This implies

  1. using bracket-terminal 0.8.1 which does not rely on ctrlc
  2. using bracket-random 0.8.0 and rand 0.7.3; newer rand has a transitive dependency through rand_core on getrandom > 0.2.0, see Comment on WASM targets where getrandom should never work rust-random/getrandom#152 , and therefore rolling back to bracket-noise 0.8.1

@vks
Copy link
Contributor

vks commented Feb 21, 2021

To fix the build failure with Rand, you might have to enable the js feature for getrandom 0.2 if you are using the wasm32-unknown-unknown target. Alternatively, you can use the wasm32-unknown-emscripten or wasm32-wasi target.

@justinbowes
Copy link
Contributor

justinbowes commented Feb 21, 2021

Right. So the rand requires js feature issue might properly go against https://github.com/amethyst/rustrogueliketutorial/blob/master/book/src/webbuild.md

The dependency on ctrlc would however block in any case.

@singalen
Copy link
Contributor Author

Looks like rand can be built without getrandom. Who needs a hardware random seeds in a roguelike game?
ctrlc probably needs to be a bracket-terminal option.

@vks
Copy link
Contributor

vks commented Feb 22, 2021

You need getrandom to initialize your RNG. It's not about hardware support.

@thebracket thebracket self-assigned this Feb 22, 2021
@thebracket thebracket added the bug Something isn't working label Feb 22, 2021
@thebracket
Copy link
Collaborator

Starting work on this now.

@singalen
Copy link
Contributor Author

You need getrandom to initialize your RNG. It's not about hardware support.

https://crates.io/crates/getrandom:

A Rust library for retrieving random data from (operating) system source. It is assumed that system always provides high-quality cryptographically secure random data, ideally backed by hardware entropy sources.

While that's what it does, what we get as a package is too much and is outright harmful. This is how node.js dependencies turn into a supermassive black hole.
I hope this case can be dealt with through a careful use of crate features and platform-specific dependencies .

thebracket added a commit that referenced this issue Feb 22, 2021
…d the rand dev-dependency in bracket-terminal no longer uses getrandom. This makes it build on my local system.
thebracket added a commit that referenced this issue Feb 22, 2021
@thebracket
Copy link
Collaborator

The wasm_fix branch has code that works now. I'm looking to see how much pain it will be to use getrandom in normal builds and just use the seconds since epoch in wasm builds. Having some odd behavior trying to just change default-features based on an arch tag in cargo.

@thebracket
Copy link
Collaborator

Apparently, I'm going to have some pain with having rand in two platforms with different feature sets. The new "resolver" fixes it, but makes bracket-random nightly only at present; I don't want to do that, so I'll stick with the seconds-since-epoch approach. It really shouldn't be a problem if a game seed isn't crypto-sound.

@thebracket thebracket mentioned this issue Feb 22, 2021
thebracket added a commit that referenced this issue Feb 22, 2021
* #194 - Seed the RNG with time since unix epoch to avoid needing getrandom().

* #194 (Note that this breaks crossterm for now). Ctrlc is optional, and the rand dev-dependency in bracket-terminal no longer uses getrandom. This makes it build on my local system.

* #194 - Crossterm feature flag is now cross_term, and ctrlc is limited to the features curses/cross_term.

* #194 - Stop repeating myself in the cargo toml file.

* Version number bump

* Run cargo fmt
@thebracket
Copy link
Collaborator

Ok, I've confirmed that all tests and examples build on wasm32 and native on my system. I'll mark this issue "pending confirmation" if someone else would like to test for me. Then I'll close it/push a crate version bump. Thanks!

@thebracket thebracket added the awaiting_confirmation Believed fix, waiting for testing label Feb 22, 2021
@justinbowes
Copy link
Contributor

justinbowes commented Feb 23, 2021

I'm a Rust neophyte, but:

cargo clean
rm Cargo.lock

# Set dependency: 
# [dependencies] 
# rltk = { git = "https://github.com/amethyst/bracket-lib" }

cargo build --release --target wasm32-unknown-unknown
wasm-bindgen ...

The build completes successfully. 🥇

However, running it, I get a panic in the web console:

Uncaught (in promise) RuntimeError: unreachable
    at __rust_start_panic (http://localhost:8080/rltktut_bg.wasm:wasm-function[1622]:0x82f9f)
    at rust_panic (http://localhost:8080/rltktut_bg.wasm:wasm-function[1190]:0x80ee6)
    at std::panicking::rust_panic_with_hook::h4f753dc70b771d8e (http://localhost:8080/rltktut_bg.wasm:wasm-function[733]:0x75179)
    at std::panicking::begin_panic::{{closure}}::hce5f3f65f96f2f2a (http://localhost:8080/rltktut_bg.wasm:wasm-function[1177]:0x80ce1)
    at std::sys_common::backtrace::__rust_end_short_backtrace::h706c23cb5ca53ba5 (http://localhost:8080/rltktut_bg.wasm:wasm-function[1138]:0x80629)
    at std::panicking::begin_panic::h500a2937ff20cfd7 (http://localhost:8080/rltktut_bg.wasm:wasm-function[1176]:0x80cb4)
    at std::sys::wasm::time::SystemTime::now::h780c8e91d0f652ad (http://localhost:8080/rltktut_bg.wasm:wasm-function[1449]:0x827da)
    at std::time::SystemTime::now::h92037ca9d5c1afb7 (http://localhost:8080/rltktut_bg.wasm:wasm-function[1563]:0x82d16)
    at bracket_random::random::RandomNumberGenerator::new::hf0a76cba565441ab (http://localhost:8080/rltktut_bg.wasm:wasm-function[744]:0x75a2d)
    at rltktut::map::Map::new_map_rooms_and_corridors::h1d98ee7f801ac015 (http://localhost:8080/rltktut_bg.wasm:wasm-function[144]:0x1fcde)

@justinbowes
Copy link
Contributor

justinbowes commented Feb 23, 2021

This may be relevant: rust-lang/rust#48564 (comment)

((I'll be happy to contribute with PRs once I get my head around Rust a little more. Trying to help however I can at the moment.)

@thebracket
Copy link
Collaborator

I'll dig into it some more, my testing didn't quite get that far (which was why I'd left it open). It's hard to believe that SystemTime::now breaks on WASM, but I guess there's a sandboxing/security problem with just exposing it. I'm pretty sure that the issue you linked will help me resolve that. Thanks!

@thebracket
Copy link
Collaborator

The patch I just committed is now working for me. The results of a WASM build: http://bfnightly.bracketproductions.com/sprites-23-02-2021/ (sprite movement is random via bracket-random).

@justinbowes
Copy link
Contributor

Confirming your findings: wasm32-unknown-unknown builds and runs my project successfully for me against current master, aabfa49. Since I intend to get there eventually, I rebuilt https://github.com/amethyst/rustrogueliketutorial and tried out chapter 74, assuming that it exercises more features. That was also fine.

Excellent! Thank you.

@vks
Copy link
Contributor

vks commented Feb 24, 2021

@thebracket Why don't you enable the getrandom/js feature instead?

@singalen
Copy link
Contributor Author

Removing a whole target system, and reducing the module portability because of implementation convenience, is not a responsible behavior towards community.

@thebracket
Copy link
Collaborator

I think I have some ideas to find a middle-ground here. (With that said, I will give the warning that if you really need crypto-strength randomness, this library may not be a good choice for you - xorshift has lots of nice properties, but I'm really not qualified to judge its entropy scores). I'm also not a huge fan of renaming the crossterm feature, but I think I should have done that one anyway - Cargo gets pretty limiting when your feature name matches a dependency.

So I'll keep this issue open and not push a crate version until I have a somewhat better solution in place.

thebracket added a commit that referenced this issue Mar 1, 2021
…n an architecture that doesn't support that syscall, falls back to time since start. On WASM, it queries your WASM host for either the browser or Node RNG functions. If they are available, it uses them to generate a good seed. If that isn't available, it falls back to using js time functions.
thebracket added a commit that referenced this issue Mar 1, 2021
…or crossterm. You don't have to change the name if you rely on the parent library, only if you rely on bracket_terminal directly.
@thebracket
Copy link
Collaborator

Ok, I've updated this again (hopefully the last one before I do a crate upload):

  • The cross_term feature is still crossterm is you use RLTK or bracket-lib directly. You only need to change the feature name if you are using bracket-terminal directly. That should minimize the impact of the feature change.
  • On a non-WASM ("native") build, bracket-random first tries to use getrandom. If for some reason your platform doesn't provide a good answer, it falls back to the time since startup. If you don't have a clock OR an RNG, it can't help you - use the with_seed call.
  • On a WASM build, it uses some code from getrandom (not getrandom itself, there's difficulties pulling in a crate with different feature flags by target at this time). It tries to detect if you are using Node, and uses Node's crypto system to get a seed. If that isn't available, it looks to see if your browser can provide a good seed - and uses that. If your browser isn't providing that either, it falls back to JS time. Again, if none of those can help you - use with_seed and provide one!

It works on my test builds (the tutorial, and http://bfnightly.bracketproductions.com/sprites-23-02-2021/ ). If anyone could give it a whirl before I publish, I'd appreciate it.

@justinbowes
Copy link
Contributor

On 927d229 in my project, all of the following now build and run. I lack a test environment for other combinations.

  • rltk/default
    • wasm32
    • x86_64-pc-windows-msvc
  • rltk/crossterm
    • x86_64-pc-windows-msvc
    • x86_64-unknown-linux-musl
  • rltk/curses
    • x86_64-pc-windows-msvc
    • x86_64-unknown-linux-musl
  • rltk/amethyst_engine_vulkan
    • x86_64-pc-windows-msvc

@thebracket
Copy link
Collaborator

I forgot to close this one - it's working on everything I've test it with.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
awaiting_confirmation Believed fix, waiting for testing bug Something isn't working
Projects
None yet
Development

No branches or pull requests

4 participants