Skip to content

Conversation

@lrlna
Copy link
Contributor

@lrlna lrlna commented Jan 5, 2019

Hello!

I am using this module in a library that will be compiled to WASM. The current version doesn't quite work, as it makes system calls (get_hostname() and libc::getpid() specifically).
You can try this yourself with cargo build --target wasm32-unknown-unknown.

This PR adds two replacement functions that will only be used in case of WASM, and in all other cases the current implementation will stay. The WASM-compatible functions just generate random numbers for machine id and process id; it's done similarly in the js version of bson.

Thank you for reviewing o/ !

@zonyitoo zonyitoo requested a review from kyeah January 6, 2019 02:16
@zonyitoo
Copy link
Contributor

zonyitoo commented Jan 6, 2019

@kyeah Please review these commits.

@saghm
Copy link
Contributor

saghm commented Jan 9, 2019

It's worth noting that MongoDB drivers as a whole are moving towards implementations independent of the hostname and PID, as described here. It might be worth just updating the implementation to follow that spec, which both saves us from needing to use conditional compilation (as the implementation would work for both standard Rust and wasm) and would bring us in line with what other drivers are doing.

@lrlna
Copy link
Contributor Author

lrlna commented Jan 9, 2019

Thanks @saghm !

If everyone is okay with using just the random bytes and having one compilation, I will gladly change this PR to accommodate that.

@kyeah
Copy link
Contributor

kyeah commented Jan 9, 2019

yep, let's move to the new spec with random initialized bytes. thanks @saghm + @lrlna, sorry for the delayed response!

@lrlna
Copy link
Contributor Author

lrlna commented Jan 11, 2019

@kyeah not delayed at all, don't worry!

let me fix this PR to just have the one compilation that uses random bytes.

@lrlna
Copy link
Contributor Author

lrlna commented Jan 14, 2019

closing in favour of #114 !

@lrlna lrlna closed this Jan 14, 2019
@lrlna lrlna deleted the wasm-suport branch February 27, 2020 14:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants