Skip to content
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

feat(wasm-support): add wasm support #351

Open
wants to merge 16 commits into
base: main
Choose a base branch
from

Conversation

irvingoujAtDevolution
Copy link

Hi @Eugeny,

As mentioned in the previous pull request, it seems like you would prefer a complete PR for the entire integration. Hence, I have merged the previous two pull requests into one, containing the full integration. Please take a look!

Key Changes:

  1. Introduced the russh-util crate, and sealed the runtime with conditional compilation: Tokio for non-WASM, and wasm-bindgen for WASM.
  2. Sealed the time functionality in russh-util: using std::time for non-WASM, and chrono for WASM.
  3. Updated cryptovec, encapsulating the differences for memset, memcpy, mlock, and munlock into separate files, gated behind feature flags.
  4. Conditionally excluded the server module from compiling for WASM. If anyone needs it in the future, they can follow a similar approach to the one I used.
  5. Restructured the known_hosts module and gated it behind a compilation flag, as the WASM environment typically lacks a file system.
  6. Made various minor changes to the project structure to improve maintainability.
  7. Added server-specific cfg and cfg_attr attributes to handle platform differences in usage.

Let me know if any further modifications are needed!

commit 686cd89
Author: irving ou <[email protected]>
Date:   Fri Sep 20 09:29:24 2024 -0400

    update cargo.toml

commit 25c0847
Author: irving ou <[email protected]>
Date:   Thu Sep 19 14:54:43 2024 -0400

    Update time.rs

commit 0d7ad5c
Author: irving ou <[email protected]>
Date:   Thu Sep 19 14:51:45 2024 -0400

    fmt

commit c9667a6
Author: irving ou <[email protected]>
Date:   Thu Sep 19 14:50:56 2024 -0400

    allow dead code

commit e1e9e8c
Author: irving ou <[email protected]>
Date:   Thu Sep 19 14:45:38 2024 -0400

    Create runtime.rs

commit 77bea62
Author: irving ou <[email protected]>
Date:   Thu Sep 19 14:45:03 2024 -0400

    Add comments

commit 2cf485c
Author: irving ou <[email protected]>
Date:   Thu Sep 19 14:44:03 2024 -0400

    feat(wasm): Add util creates for runtime,future and time primitives for wasm
#[derive(Debug)]
pub struct JoinError {
#[cfg(not(target_arch = "wasm32"))]
inner: tokio::task::JoinError,

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see that the current approach with JoinHandle seems to align with your preference. However, I’d like to propose a discussion on possibly using a one-shot channel as a unified simulation for the handle across targets. This would significantly reduce the code complexity and maintain a consistent type between WASM and non-WASM environments. By doing so, we could simplify the overall implementation while still achieving the desired functionality.

@Eugeny
Copy link
Owner

Eugeny commented Sep 20, 2024

Thank you for your work! I've added some changes:

  • Simplified spawn to always use a channel as you suggested
  • Removed panic on the JoinHandle channel (tokio tasks do not panic if you do not await them)
  • Un-gated russh_keys::agent
  • Unified the duplicate write_kex impls
  • Removed russh::Error::TokioJoin
  • Added a CI build

If all looks good to you, this PR is good to go

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.

2 participants