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

make wasm32-* compatible #1

Merged
merged 4 commits into from
Aug 27, 2024
Merged

Conversation

DougAnderson444
Copy link
Contributor

@DougAnderson444 DougAnderson444 commented Aug 12, 2024

Current config does not allow for wasm32-* target builds.

This PR puts ssh-* behind a ssh feature flag, and only for non-wasm32 targets. uses ssh-key without default-features to enable wasm32 builds.

[target.'cfg(target_arch = "wasm32")'.dependencies]
ssh-key = { version = "0.6", default-features = false, features = [
  "alloc",
  "ecdsa",
  "ed25519",
] }

[target.'cfg(not(target_arch = "wasm32"))'.dependencies]
ssh-key = { version = "0.6", features = ["alloc", "crypto", "ed25519"] }

Note that the crypto feature of ssh-key seems to still break wasm32 builds (I'm looking at you, p384), but there are many algs that aren't being used so this feature is paired down to only the 2 that are used.

@DougAnderson444
Copy link
Contributor Author

This is blocking completion of cryptidtech/multikey#2

@DougAnderson444
Copy link
Contributor Author

DougAnderson444 commented Aug 25, 2024

Improved PR:

  • sets ssh-key default-features to false (for no std) instead of ssh feature flag
  • needed to switch SSH Error to account for lack of std Error

This way, wasm32 targets get the SSH

@DougAnderson444 DougAnderson444 changed the title put ssh-* behind a feature flag, for non(wasm32) only make wasm32-* compatible Aug 25, 2024
@DougAnderson444
Copy link
Contributor Author

@dhuseby this is ready for review now

Copy link
Member

@dhuseby dhuseby left a comment

Choose a reason for hiding this comment

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

LGTM

@dhuseby dhuseby merged commit 8964981 into cryptidtech:main Aug 27, 2024
1 check passed
@DougAnderson444 DougAnderson444 deleted the wasm32 branch August 27, 2024 14:58
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