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

fix: wasm_opt #114

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

Conversation

ChenKS12138
Copy link
Contributor

@ChenKS12138 ChenKS12138 commented Feb 19, 2022

  1. As @PiotrSikora says, in order to runwasm_opt, a dynamic library needs to be installed (for Linux and mac os). Update wasm-opt to version_97. #91 (comment)
    And the cache folder's structure will be as follow.

image

  1. update wasm_opt version from version_97 to version_105

@ChenKS12138
Copy link
Contributor Author

ChenKS12138 commented Feb 19, 2022

Oops, some checks are stills be not successful, IMHO maybe examples/markdown is not a suitable example for cargo-wasi, we need to remove it, as @alexcrichton says

the wasm32-wasi target is not compatible with wasm-bindgen

rustwasm/wasm-bindgen#2554 (comment)

Or, we should allow users to pass and override the target argument, something like that. (It works well to compile examples/markdown if I have changed the target)

cargo wasi build -p markdown --release --verbose --target wasm32-unknown-unknown

cargo-wasi/src/lib.rs

Lines 82 to 94 in 6d2e502

// TODO: figure out when these flags are already passed to `cargo` and skip
// passing them ourselves.
cargo.arg("--target").arg("wasm32-wasi");
cargo.arg("--message-format").arg("json-render-diagnostics");
for arg in args {
if let Some(arg) = arg.to_str() {
if arg.starts_with("--verbose") || arg.starts_with("-v") {
config.set_verbose(true);
}
}
cargo.arg(arg);
}

@ChenKS12138 ChenKS12138 force-pushed the fix-wasm_opt branch 2 times, most recently from 058155e to 3364dab Compare February 23, 2022 14:43
@ChenKS12138
Copy link
Contributor Author

  1. As @PiotrSikora says, in order to runwasm_opt, a dynamic library needs to be installed (for Linux and mac os). Update wasm-opt to version_97. #91 (comment)
    And the cache folder's structure will be as follow.

image

  1. bump wasm_opt version from version_97 to version_105
  1. Installing target wasm32-unknown-unknown when compiling example/markdown on CI.

@alexcrichton
Copy link
Member

Sorry there's a good number of changes here and I don't have a ton of time to review this right now. If wasm-opt-the-builtin-version isn't working well I'd recommend disabling it and then manually running it afterwards.

@ChenKS12138
Copy link
Contributor Author

I have removed the code that updating wasm-opt version, and I believe this is the minimum code changes to let CI checks pass.

There is no problem with wasm-opt-the-builtin-version, but an essential library (libbinaryen.dylib/libbinaryen.a) is not installed and linked.

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