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

Update seahorse to latest commit, which includes a wasm fix #90

Merged
merged 1 commit into from Nov 30, 2022

Conversation

mcintyre94
Copy link
Contributor

  • Update Seahorse to the latest current commit. This is pre-release, but fixes the issue that stopped it compiling in wasm
  • Slight change to lib.rs because the compile signature has changed a bit. We pass a PathBuf in, it's optional but defaults to finding the current directory - which is a runtime error in wasm
  • I've bumped the toolchain version. I was getting an error that 2022-06-27 doesn't exist, and it doesn't seem to be in releases. Not sure what's going on there, but I've bumped to the latest version

Note that this still doesn't give us the multi-file python imports feature that seahorse now has. We're still using compile which will only work with a single Python source file. But it brings us up to date with the latest seahorse features/fixes within that existing constraint.

Fixes #75, #86

@acheroncrypto
Copy link
Member

Nice! I think it's good that we use the latest stable version anyway but have you tried to compile other wasm packages in the repo? We currently don't have Github actions checks and I'm afraid it could break other builds.

I'll merge if you can confirm other packages compile with these versions.

@mcintyre94
Copy link
Contributor Author

I haven't built most of these before so might have done something wrong, but I've tried to build them all with this change.
I also used the script from this awesome stackoverflow answer to check for file size differences

rustfmt/rustfmt-wasm

I got an error building this, but it seems to have its own toolchain and the error refers to that version. I haven't built this myself before so not sure if there's something I'm doing wrong

$ cd rustfmt/rustfmt-wasm/
$ wasm-pack build
Error: Error during execution of `cargo metadata`: info: syncing channel updates for 'nightly-2018-09-07-aarch64-apple-darwin'
info: latest update on 2018-09-07, rust version 1.30.0-nightly (a8c11d216 2018-09-06)
error: target 'aarch64-apple-darwin' not found in channel.  Perhaps check https://doc.rust-lang.org/nightly/rustc/platform-support.html for available targets

solana-cli

no error, output is a bit smaller

$ ../../diff-sizes.sh --cached HEAD | grep solana-cli
-1      wasm/pkgs/solana-cli/package.json
2168    wasm/pkgs/solana-cli/solana_cli_wasm_bg.js
-93642  wasm/pkgs/solana-cli/solana_cli_wasm_bg.wasm
0       wasm/pkgs/solana-cli/solana_cli_wasm_bg.wasm.d.ts

solana-client

I got an error saying this isn't the right crate-type for wasm

$ wasm-pack build
Error: crate-type must be cdylib to compile to wasm32-unknown-unknown. Add the following to your Cargo.toml file:

[lib]
crate-type = ["cdylib", "rlib"]

After adding that it built with no error
Doesn't seem to be a built version currently checked in to compare against

spl-token-cli

no error, a bit smaller too

$ ../../diff-sizes.sh --cached HEAD | grep spl-token-cli
2177    wasm/pkgs/spl-token-cli/spl_token_cli_wasm_bg.js
-85504  wasm/pkgs/spl-token-cli/spl_token_cli_wasm_bg.wasm
0       wasm/pkgs/spl-token-cli/spl_token_cli_wasm_bg.wasm.d.ts

sugar-cli

no error, slightly bigger

$ ../../diff-sizes.sh --cached HEAD | grep sugar-cli    
0       wasm/pkgs/sugar-cli/sugar_cli_wasm_bg.js
16653   wasm/pkgs/sugar-cli/sugar_cli_wasm_bg.wasm
0       wasm/pkgs/sugar-cli/sugar_cli_wasm_bg.wasm.d.ts

Copy link
Member

@acheroncrypto acheroncrypto left a comment

Choose a reason for hiding this comment

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

Thank you! I think this is good to go because rustfmt depends on an old version of rustc which your machine doesn't have support for.

@acheroncrypto acheroncrypto merged commit 35cefc0 into solana-playground:master Nov 30, 2022
@mcintyre94 mcintyre94 deleted the seahorse-upgrade branch November 30, 2022 14:36
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.

need to update the seahorse version for writing Smart Contracts in Seahorse
2 participants