Fix binaryen toolchain URL

[matheus23]

This should fix the tests, as a follow-up from #937.

[serprex]

@drager

[matheus23]

Yeah this is not everything. Macos is now still broken.

---- build::build_force stdout ----
Created fixture at /Users/runner/work/wasm-pack/wasm-pack/target/t/.tmpaWHPLW/wasm-pack
thread 'build::build_force' panicked at 'called `Result::unwrap()` on an `Err` value: ErrorMessage { msg: "received a bad HTTP status code (404) when requesting https://github.com/WebAssembly/binaryen/releases/download/version_108/binaryen-version_108-x86_64-apple-darwin.tar.gz" }

It refers to binaryen-version_108-x86_64-apple-darwin.tar.gz but only binaryen-version_108-x86_64-macos.tar.gz exists.

[matheus23]

How about we add a test that checks that the prebuild_url is non-404 for the whole matrix of values it could be provided with?

[serprex]

Relevant PR: https://github.com/WebAssembly/binaryen/pull/2777

[matheus23]

Okay, I guess 32-bit x86 was never supported then. Should we just error out if the target == x86 32-bit?

[serprex]

Yes

[matheus23]

Now I'm getting

dyld: Library not loaded: @rpath/libbinaryen.dylib
  Referenced from: /Users/runner/work/wasm-pack/wasm-pack/target/test_cache/wasm-opt-1b12e8f5a5dc321a/wasm-opt
  Reason: image not found

This feels too hard to pull off for me :/ I'd appreciate and and all help!

[printfn]

I've been doing some digging into the wasm-opt dyld errors, and I think I know how this bug can be fixed! The issue seems to be that the binary-install crate isn't downloading the dylib correctly, which I was able to hack around by setting DYLD_LIBRARY_PATH.

Here's a patch that can be used on top of this PR, which "fixes" the issue on macOS:

diff --git a/src/install/mod.rs b/src/install/mod.rs
index 1a77926..612c6da 100644
--- a/src/install/mod.rs
+++ b/src/install/mod.rs
@@ -152,7 +152,7 @@ pub fn download_prebuilt(
             }
         }
         Tool::WasmOpt => {
-            let binaries = &["wasm-opt"];
+            let binaries = &["wasm-opt", "libbinaryen"];
             match cache.download(install_permitted, "wasm-opt", binaries, &url)? {
                 Some(download) => Ok(Status::Found(download)),
                 // TODO(ag_dubs): why is this different? i forget...
diff --git a/src/wasm_opt.rs b/src/wasm_opt.rs
index d3ad4c6..cd8944e 100644
--- a/src/wasm_opt.rs
+++ b/src/wasm_opt.rs
@@ -40,6 +40,7 @@ pub fn run(
         let tmp = path.with_extension("wasm-opt.wasm");
         let mut cmd = Command::new(&wasm_opt_path);
         cmd.arg(&path).arg("-o").arg(&tmp).args(args);
+        cmd.env("DYLD_LIBRARY_PATH", &wasm_opt_path.parent().unwrap());
         child::run(cmd, "wasm-opt")?;
         std::fs::rename(&tmp, &path)?;
     }

To fully fix this bug I think it'll be necessary to make a PR to the binary-install crate, so it can correctly extract the dylib to a path where it can be found automatically. Would that approach make sense?

---

Separately from all that, I'd also suggest applying this patch, to download the aarch64 version of binaryen on macos. It avoids using Rosetta needlessly.

diff --git a/src/install/mod.rs b/src/install/mod.rs
index ab4f6ff..1a77926 100644
--- a/src/install/mod.rs
+++ b/src/install/mod.rs
@@ -180,9 +180,9 @@ pub fn prebuilt_url_for(
     let target = match (os, arch, tool) {
         (Os::Linux, Arch::X86_64, Tool::WasmOpt) => "x86_64-linux",
         (Os::Linux, Arch::X86_64, _) => "x86_64-unknown-linux-musl",
-        (Os::MacOS, Arch::X86, _) => bail!("Unrecognized target!"),
-        (Os::MacOS, _, Tool::WasmOpt) => "x86_64-macos",
-        (Os::MacOS, _, _) => "x86_64-apple-darwin",
+        (Os::MacOS, Arch::X86_64, Tool::WasmOpt) => "x86_64-macos",
+        (Os::MacOS, Arch::X86_64, _) => "x86_64-apple-darwin",
+        (Os::MacOS, Arch::AArch64, Tool::WasmOpt) => "arm64-macos",
         (Os::Windows, Arch::X86_64, Tool::WasmOpt) => "x86_64-windows",
         (Os::Windows, Arch::X86_64, _) => "x86_64-pc-windows-msvc",
         _ => bail!("Unrecognized target!"),

[drager]

I've been doing some digging into the wasm-opt dyld errors, and I think I know how this bug can be fixed! The issue seems to be that the binary-install crate isn't downloading the dylib correctly, which I was able to hack around by setting DYLD_LIBRARY_PATH.

Here's a patch that can be used on top of this PR, which "fixes" the issue on macOS:

diff --git a/src/install/mod.rs b/src/install/mod.rs
index 1a77926..612c6da 100644
--- a/src/install/mod.rs
+++ b/src/install/mod.rs
@@ -152,7 +152,7 @@ pub fn download_prebuilt(
             }
         }
         Tool::WasmOpt => {
-            let binaries = &["wasm-opt"];
+            let binaries = &["wasm-opt", "libbinaryen"];
             match cache.download(install_permitted, "wasm-opt", binaries, &url)? {
                 Some(download) => Ok(Status::Found(download)),
                 // TODO(ag_dubs): why is this different? i forget...
diff --git a/src/wasm_opt.rs b/src/wasm_opt.rs
index d3ad4c6..cd8944e 100644
--- a/src/wasm_opt.rs
+++ b/src/wasm_opt.rs
@@ -40,6 +40,7 @@ pub fn run(
         let tmp = path.with_extension("wasm-opt.wasm");
         let mut cmd = Command::new(&wasm_opt_path);
         cmd.arg(&path).arg("-o").arg(&tmp).args(args);
+        cmd.env("DYLD_LIBRARY_PATH", &wasm_opt_path.parent().unwrap());
         child::run(cmd, "wasm-opt")?;
         std::fs::rename(&tmp, &path)?;
     }

To fully fix this bug I think it'll be necessary to make a PR to the binary-install crate, so it can correctly extract the dylib to a path where it can be found automatically. Would that approach make sense?

Separately from all that, I'd also suggest applying this patch, to download the aarch64 version of binaryen on macos. It avoids using Rosetta needlessly.

diff --git a/src/install/mod.rs b/src/install/mod.rs
index ab4f6ff..1a77926 100644
--- a/src/install/mod.rs
+++ b/src/install/mod.rs
@@ -180,9 +180,9 @@ pub fn prebuilt_url_for(
     let target = match (os, arch, tool) {
         (Os::Linux, Arch::X86_64, Tool::WasmOpt) => "x86_64-linux",
         (Os::Linux, Arch::X86_64, _) => "x86_64-unknown-linux-musl",
-        (Os::MacOS, Arch::X86, _) => bail!("Unrecognized target!"),
-        (Os::MacOS, _, Tool::WasmOpt) => "x86_64-macos",
-        (Os::MacOS, _, _) => "x86_64-apple-darwin",
+        (Os::MacOS, Arch::X86_64, Tool::WasmOpt) => "x86_64-macos",
+        (Os::MacOS, Arch::X86_64, _) => "x86_64-apple-darwin",
+        (Os::MacOS, Arch::AArch64, Tool::WasmOpt) => "arm64-macos",
         (Os::Windows, Arch::X86_64, Tool::WasmOpt) => "x86_64-windows",
         (Os::Windows, Arch::X86_64, _) => "x86_64-pc-windows-msvc",
         _ => bail!("Unrecognized target!"),

Thanks for investigating! And yes, I think it makes sense to send a PR to binary-install. Feel free to do so. Would appreciate it!