Skip to content

Install to subdirs#21

Merged
drager merged 7 commits intodrager:masterfrom
printfn:install-to-subdirs
Oct 30, 2022
Merged

Install to subdirs#21
drager merged 7 commits intodrager:masterfrom
printfn:install-to-subdirs

Conversation

@printfn
Copy link
Contributor

@printfn printfn commented Oct 27, 2022

As discussed in wasm-pack#1163, this PR adds the ability to extract binaries (and libraries) to specific subdirectories, which is needed for dynamic linking on macOS.

Specifically, the macOS tarballs at https://github.com/WebAssembly/binaryen/releases/tag/version_110 contain this folder structure:

$ tar -tf ~/Downloads/binaryen-version_110-arm64-macos.tar.gz
binaryen-version_110/
binaryen-version_110/bin/
binaryen-version_110/include/
binaryen-version_110/lib/
binaryen-version_110/lib/libbinaryen.dylib
binaryen-version_110/include/binaryen-c.h
binaryen-version_110/include/wasm-delegations.def
binaryen-version_110/bin/wasm-split
binaryen-version_110/bin/wasm-ctor-eval
binaryen-version_110/bin/wasm-reduce
binaryen-version_110/bin/wasm-metadce
binaryen-version_110/bin/wasm-shell
binaryen-version_110/bin/wasm-fuzz-types
binaryen-version_110/bin/binaryen-unittests
binaryen-version_110/bin/wasm-emscripten-finalize
binaryen-version_110/bin/wasm-as
binaryen-version_110/bin/wasm-opt
binaryen-version_110/bin/wasm-dis
binaryen-version_110/bin/wasm2js

To install wasm-opt from the tarball, the dynamic linker needs to be able to find libbinaryen.dylib at the relative path ../lib/libbinaryen.dylib, so we need to be able to install it to an appropriate subdirectory that mimics the structure in the .tar.gz file.

To implement this, I've added a feature to this crate so that passing the strings "bin/wasm-opt" and "lib/libbinaryen.dylib" to the Cache::download method will extract those files to matching subdirectories inside the cache directory, which can then be invoked as normal with Download::binary. This should be fully backwards-compatible since the subdirectories are only created when specifically passed in by the user.

This is how this can be used in wasm-pack:

--- a/src/install/mod.rs
+++ b/src/install/mod.rs
@@ -136,6 +136,10 @@ pub fn download_prebuilt(
             }
         }
         Tool::WasmOpt => {
-            let binaries = &["wasm-opt"];
+            let binaries: &[&str] = match Os::get()? {
+                Os::MacOS => &["bin/wasm-opt", "lib/libbinaryen.dylib"],
+                Os::Linux => &["bin/wasm-opt"],
+                Os::Windows => &["bin/wasm-opt.exe"],
+            };
             match cache.download(install_permitted, "wasm-opt", binaries, &url)? {
                 Some(download) => Ok(Status::Found(download)),
--- a/src/wasm_opt.rs
+++ b/src/wasm_opt.rs
@@ -27,7 +27,7 @@ pub fn run(
         }
     };
 
-    let wasm_opt_path = wasm_opt.binary(&Tool::WasmOpt.to_string())?;
+    let wasm_opt_path = wasm_opt.binary("bin/wasm-opt")?;
     PBAR.info("Optimizing wasm binaries with `wasm-opt`...");
 
     for file in out_dir.read_dir()? {

I've written a working version in this test commit (or this branch), which is based on the wasm-pack#1163 PR. There's some error-handling-related hacks because wasm-pack still uses failure instead of anyhow (which this crate seems to have switched to in 2020, but never released to crates.io?), but apart from that it seems to work well.


Make sure these boxes are checked! 📦✅

  • You have the latest version of rustfmt installed
$ rustup component add rustfmt-preview --toolchain nightly
  • You ran cargo fmt on the code base before submitting
  • You reference which issue is being closed in the PR text

✨✨ 😄 Thanks so much for contributing to binary-install! 😄 ✨✨

Copy link
Owner

@drager drager left a comment

Choose a reason for hiding this comment

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

Thanks! I think it looks good. Just need to figure out why the pipelines were cancelled and I think we're good to go.

@printfn
Copy link
Contributor Author

printfn commented Oct 28, 2022

It looks like the pipelines are failing with

##[warning]An image label with the label vs2017-win2016 does not exist.
,##[error]The remote provider was unable to process the request.
Started: Just now
Duration: 46s

and

##[warning]An image label with the label Ubuntu16 does not exist.
,##[error]The remote provider was unable to process the request.
Started: Just now
Duration: 1m 15s

@drager
Copy link
Owner

drager commented Oct 30, 2022

Thanks a lot!

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