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

wasm-opt binary not found, even though it's in PATH #1062

Closed
winterqt opened this issue Sep 26, 2021 · 11 comments
Closed

wasm-opt binary not found, even though it's in PATH #1062

winterqt opened this issue Sep 26, 2021 · 11 comments

Comments

@winterqt
Copy link

🐛 Bug description

My copy of wasm-opt isn't found by wasm-pack, even though it is in my PATH.

🤔 Expected Behavior

It should've found it.

👟 Steps to reproduce

Run wasm-pack with --mode no-install.

🌍 Your environment

Include the relevant details of your environment.
wasm-pack version: 0.9.1
rustc version: 1.55.0

Additional context

Even the which crate that's used to find the binary agrees with me, so I'm really not sure what could be wrong...

println!("{:?}", which::which("wasm-opt"));
Ok("/nix/store/jj9c673wv6pfpwwrcqcmlqajr88bavw1-binaryen-101/bin/wasm-opt")
@TLATER
Copy link

TLATER commented Jul 9, 2022

Run ldd /nix/store/jj9c673wv6pfpwwrcqcmlqajr88bavw1-binaryen-101/bin/wasm-opt. You'll see it's not that the binary is missing, but rather its interpreter is wrong. This NixOS wiki article covers that: https://nixos.wiki/wiki/Packaging/Binaries

That said, when using wasm-pack on NixOS, apparently just having this package in your build environment will allow it to resolve wasm-opt: https://search.nixos.org/packages?channel=22.05&show=binaryen&from=0&size=50&sort=relevance&type=packages&query=binaryen

@winterqt
Copy link
Author

winterqt commented Jul 9, 2022

That shouldn't be an issue here, we don't package Binaryen binaries in a way that would cause that (we build from source using our toolchains etc. so the paths are correct).

@TLATER
Copy link

TLATER commented Jul 9, 2022

Ah, hrm, my bad. I came here to see how others were getting around the binaryen auto-install, and assumed you'd fallen for the classic unclear mising interpreter error message.

If it's a data point for you, I can't reproduce this, using wasm-pack 0.10.2 and binaryen 102 from nixpkgs. They appear to be unpatched downstream, so perhaps this has been resolved by now?

@nakamurarts
Copy link

I am probably in a similar situation.
I tried to build a hello-wasm project following MDN tutorial. I had already installed wasm-opt beforehand using Scoop.

OS: Windows 10 (but also on Fedora 37)

Version info:

PS hello-wasm> wasm-pack --version
wasm-pack 0.11.0

PS hello-wasm> (Get-Command wasm-pack).Source
C:\Users\user\.cargo\bin\wasm-pack.exe

PS hello-wasm> wasm-opt --version
wasm-opt version 112 (version_112)

PS hello-wasm> (Get-Command wasm-opt).Source
C:\Users\user\scoop\shims\wasm-opt.exe

And I get the following error:

PS hello-wasm> wasm-pack build --target web
[INFO]: Checking for the Wasm target...
[INFO]: Compiling to Wasm...
  Finished release [optimized] target(s) in 0.03s
[WARN]: :-) origin crate has no README
[INFO]: Installing wasm-bindgen...
[INFO]: found wasm-opt at "C:\\Users\\user\\scoop\\shims\\wasm-opt.exe"
Error: C:\Users\user\scoop\shims\bin/wasm-opt.exe binary does not exist
To disable `wasm-opt`, add `wasm-opt = false` to your package metadata in your `Cargo.toml`.
Caused by: C:\Users\user\scoop\shims\bin/wasm-opt.exe binary does not exist
To disable `wasm-opt`, add `wasm-opt = false` to your package metadata in your `Cargo.toml`.

In this case, you can see that it is looking for a different path (C:\Users\user\scoop\shims\bin/wasm-opt.exe) than the path found by which ("C:\\Users\\user\\scoop\\shims\\wasm-opt.exe").

The following locations appear to be the cause:

pub fn find_wasm_opt(cache: &Cache, install_permitted: bool) -> Result<install::Status> {
// First attempt to look up in PATH. If found assume it works.
if let Ok(path) = which::which("wasm-opt") {
PBAR.info(&format!("found wasm-opt at {:?}", path));
match path.as_path().parent() {
Some(path) => return Ok(install::Status::Found(Download::at(path))),
let wasm_opt = match find_wasm_opt(cache, install_permitted)? {
install::Status::Found(path) => path,
install::Status::CannotInstall => {
PBAR.info("Skipping wasm-opt as no downloading was requested");
return Ok(());
}
install::Status::PlatformNotSupported => {
PBAR.info("Skipping wasm-opt because it is not supported on this platform");
return Ok(());
}
};
let wasm_opt_path = wasm_opt.binary("bin/wasm-opt")?;

(I think it would be better if the path to binaryen tools could be specified by command line options)

ivan-aksamentov added a commit to nextstrain/nextclade that referenced this issue Mar 21, 2023
There seems to be an issue (or an expected change) in wasm-pack 0.11.0: rustwasm/wasm-pack#1062, where a path to`wasm-opt` is not detected correctly. This breaks our build. Everything seems to be working well on wasm-pack 0.10.3, so let's freeze the version for the time being.
ivan-aksamentov added a commit to nextstrain/nextclade that referenced this issue Mar 21, 2023
There seems to be an issue (or an expected change) in wasm-pack 0.11.0: rustwasm/wasm-pack#1062, where a path to`wasm-opt` is not detected correctly. This [breaks our build](https://github.com/nextstrain/nextclade/actions/runs/4474405842/jobs/7862840403). Everything seems to be working well on wasm-pack 0.10.3, so let's freeze the version for the time being.
@nakamurarts
Copy link

Changes related to the above error:
30f2b0a

@drager
Copy link
Member

drager commented Mar 21, 2023

Thanks for reporting. At a quick glance, is the problem slash differences between unix-systems and windows? /vs \? Perhaps we should make this part use Pathand resolve the path instead here: 30f2b0a#diff-1d66856d3a84fec417fa7ad785058d0a9876288ed0139b3a1d8b6e7ff3fd25b4R154

@nakamurarts
Copy link

nakamurarts commented Mar 21, 2023

No, it occurs regardless of OS (I have opened a new issue #1247 , and please see: #1247 (comment). The same problem has been confirmed on mac and linux).

The problem is caused by trying to run ${somewhere}/bin/wasm-opt with an extra /bin/ instead of the correct path ${somewhere}/wasm-opt to the installed wasm-opt.
30f2b0a#diff-d9cd988699c3224bc78800edb64a7cb60d3cae9833460f79ce2d586b9555cd83R26

When a tarball (e.g. binaryen-version_111-x86_64-linux.tar.gz) is newly downloaded, this /bin/ is necessary (in this case, binaryen-version_111-x86_64-linux/bin/wasm-opt), but if it is present in the PATH from the beginning, /bin/ is not necessary.

@jamesyoungman
Copy link

nakamurarts is correct.

I used this patch to fix the problem:

commit 40e0732abf7159f0f962e8a70bc03e8552606fc1 (HEAD -> jy/wasm-opt-locatoin, origin/jy/wasm-opt-locatoin)
Author: James Youngman <[email protected]>
Date:   Sat Mar 25 10:42:57 2023 +0000

    Use correct path name for locally-installed wasm-opt.
    
    If wasm-opt is installed in `/usr/bin/wasm-opt`, the previous code
    tried to execute `/usr/bin/bin/wasm-opt` which of course doesn't
    exist.
    
    Fixes #1062

diff --git a/src/wasm_opt.rs b/src/wasm_opt.rs
index 6e2149a..2951b23 100644
--- a/src/wasm_opt.rs
+++ b/src/wasm_opt.rs
@@ -23,7 +23,7 @@ pub fn run(cache: &Cache, out_dir: &Path, args: &[String], install_permitted: bo
         }
     };
 
-    let wasm_opt_path = wasm_opt.binary("bin/wasm-opt")?;
+    let wasm_opt_path = wasm_opt.binary("wasm-opt")?;
     PBAR.info("Optimizing wasm binaries with `wasm-opt`...");
 
     for file in out_dir.read_dir()? {

@jamesyoungman
Copy link

This problem happens, by the way, on a Debian 11.6 (i.e. bullseye which is current stable) system (running binaryen version 99-3). So, not a niche development platform, particularly.

@Liamolucko
Copy link
Contributor

A fix for this was merged in #1257, so installing a git version of wasm-pack with cargo install --git=https://github.com/rustwasm/wasm-pack should fix this.

@drager
Copy link
Member

drager commented May 11, 2023

v0.11.1 is now released:

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 a pull request may close this issue.

6 participants