From 5218be592cd4c90f8c9d13d9789e5aaf9b217efb Mon Sep 17 00:00:00 2001 From: Ashley Williams Date: Fri, 19 Jul 2019 10:21:42 -0500 Subject: [PATCH 1/3] refactor(tool): wasm-opt is a tool variant --- src/bindgen.rs | 16 ++++---- src/command/build.rs | 6 +-- src/command/test.rs | 7 +++- src/generate.rs | 11 ++++-- src/install/mod.rs | 76 +++++++++++++++++++++++++++++--------- src/install/tool.rs | 12 ++++-- src/wasm_opt.rs | 76 +++++++++----------------------------- tests/all/download.rs | 20 +++++++--- tests/all/utils/fixture.rs | 12 +++++- 9 files changed, 133 insertions(+), 103 deletions(-) diff --git a/src/bindgen.rs b/src/bindgen.rs index 13da5904..02d02dce 100644 --- a/src/bindgen.rs +++ b/src/bindgen.rs @@ -1,10 +1,9 @@ //! Functionality related to running `wasm-bindgen`. -use binary_install::Download; use child; use command::build::{BuildProfile, Target}; use failure::{self, ResultExt}; -use install; +use install::{self, Tool}; use manifest::CrateData; use semver; use std::path::{Path, PathBuf}; @@ -14,7 +13,7 @@ use std::process::Command; /// `.wasm`. pub fn wasm_bindgen_build( data: &CrateData, - bindgen: &Download, + install_status: &install::Status, out_dir: &Path, out_name: &Option, disable_dts: bool, @@ -40,7 +39,8 @@ pub fn wasm_bindgen_build( } else { "--typescript" }; - let bindgen_path = bindgen.binary("wasm-bindgen")?; + let bindgen_path = install::get_tool_path(install_status, Tool::WasmBindgen)? + .binary(&Tool::WasmBindgen.to_string())?; let mut cmd = Command::new(&bindgen_path); cmd.arg(&wasm_path) @@ -49,7 +49,7 @@ pub fn wasm_bindgen_build( .arg(dts_arg); let target_arg = build_target_arg(target, &bindgen_path)?; - if supports_dash_dash_target(&bindgen_path)? { + if supports_dash_dash_target(bindgen_path.to_path_buf())? { cmd.arg("--target").arg(target_arg); } else { cmd.arg(target_arg); @@ -85,17 +85,17 @@ fn supports_web_target(cli_path: &PathBuf) -> Result { } /// Check if the `wasm-bindgen` dependency is locally satisfied for the --target flag -fn supports_dash_dash_target(cli_path: &PathBuf) -> Result { +fn supports_dash_dash_target(cli_path: PathBuf) -> Result { let cli_version = semver::Version::parse(&install::get_cli_version( &install::Tool::WasmBindgen, - cli_path, + &cli_path, )?)?; let expected_version = semver::Version::parse("0.2.40")?; Ok(cli_version >= expected_version) } fn build_target_arg(target: Target, cli_path: &PathBuf) -> Result { - if !supports_dash_dash_target(cli_path)? { + if !supports_dash_dash_target(cli_path.to_path_buf())? { Ok(build_target_arg_legacy(target, cli_path)?) } else { Ok(target.to_string()) diff --git a/src/command/build.rs b/src/command/build.rs index 4291cddc..b1b6eb50 100644 --- a/src/command/build.rs +++ b/src/command/build.rs @@ -1,7 +1,7 @@ //! Implementation of the `wasm-pack build` command. use crate::wasm_opt; -use binary_install::{Cache, Download}; +use binary_install::Cache; use bindgen; use build; use cache; @@ -32,7 +32,7 @@ pub struct Build { pub mode: InstallMode, pub out_dir: PathBuf, pub out_name: Option, - pub bindgen: Option, + pub bindgen: Option, pub cache: Cache, pub extra_options: Vec, } @@ -366,7 +366,7 @@ impl Build { info!("Building the wasm bindings..."); bindgen::wasm_bindgen_build( &self.crate_data, - self.bindgen.as_ref().unwrap(), + &self.bindgen.as_ref().unwrap(), &self.out_dir, &self.out_name, self.disable_dts, diff --git a/src/command/test.rs b/src/command/test.rs index 8bf262d2..cc77a104 100644 --- a/src/command/test.rs +++ b/src/command/test.rs @@ -268,14 +268,17 @@ impl Test { ) } - let dl = install::download_prebuilt_or_cargo_install( + let status = install::download_prebuilt_or_cargo_install( Tool::WasmBindgen, &self.cache, &bindgen_version, self.mode.install_permitted(), )?; - self.test_runner_path = Some(dl.binary("wasm-bindgen-test-runner")?); + self.test_runner_path = match status { + install::Status::Found(dl) => Some(dl.binary("wasm-bindgen-test-runner")?), + _ => bail!("Could not find 'wasm-bindgen-test-runner'."), + }; info!("Getting wasm-bindgen-cli was successful."); Ok(()) diff --git a/src/generate.rs b/src/generate.rs index 97bb1ccf..5a385f2c 100644 --- a/src/generate.rs +++ b/src/generate.rs @@ -1,15 +1,20 @@ //! Functionality related to running `cargo-generate`. -use binary_install::Download; use child; use emoji; use failure::{self, ResultExt}; +use install::{self, Tool}; use std::process::Command; /// Run `cargo generate` in the current directory to create a new /// project from a template -pub fn generate(template: &str, name: &str, download: &Download) -> Result<(), failure::Error> { - let bin_path = download.binary("cargo-generate")?; +pub fn generate( + template: &str, + name: &str, + install_status: &install::Status, +) -> Result<(), failure::Error> { + let bin_path = install::get_tool_path(install_status, Tool::CargoGenerate)? + .binary(&Tool::CargoGenerate.to_string())?; let mut cmd = Command::new(&bin_path); cmd.arg("generate"); cmd.arg("--git").arg(&template); diff --git a/src/install/mod.rs b/src/install/mod.rs index 79999166..79c8919d 100644 --- a/src/install/mod.rs +++ b/src/install/mod.rs @@ -5,6 +5,7 @@ use binary_install::{Cache, Download}; use child; use emoji; use failure::{self, ResultExt}; +use install; use log::debug; use log::{info, warn}; use std::env; @@ -21,6 +22,27 @@ mod tool; pub use self::mode::InstallMode; pub use self::tool::Tool; +/// Possible outcomes of attempting to find/install a tool +pub enum Status { + /// Couldn't install tool because downloads are forbidden by user + CannotInstall, + /// The current platform doesn't support precompiled binaries for this tool + PlatformNotSupported, + /// We found the tool at the specified path + Found(Download), +} + +/// Handles possible installs status and returns the download or a error message +pub fn get_tool_path(status: &Status, tool: Tool) -> Result<&Download, failure::Error> { + match status { + Status::Found(download) => Ok(download), + Status::CannotInstall => bail!("Not able to find or install a local {}.", tool), + install::Status::PlatformNotSupported => { + bail!("{} does not currently support your platform.", tool) + } + } +} + /// Install a cargo CLI tool /// /// Prefers an existing local install, if any exists. Then checks if there is a @@ -32,7 +54,7 @@ pub fn download_prebuilt_or_cargo_install( cache: &Cache, version: &str, install_permitted: bool, -) -> Result { +) -> Result { // If the tool is installed globally and it has the right version, use // that. Assume that other tools are installed next to it. // @@ -41,7 +63,8 @@ pub fn download_prebuilt_or_cargo_install( if let Ok(path) = which(tool.to_string()) { debug!("found global {} binary at: {}", tool, path.display()); if check_version(&tool, &path, version)? { - return Ok(Download::at(path.parent().unwrap())); + let download = Download::at(path.parent().unwrap()); + return Ok(Status::Found(download)); } } @@ -101,7 +124,7 @@ pub fn download_prebuilt( cache: &Cache, version: &str, install_permitted: bool, -) -> Result { +) -> Result { let url = match prebuilt_url(tool, version) { Ok(url) => url, Err(e) => bail!( @@ -114,17 +137,25 @@ pub fn download_prebuilt( Tool::WasmBindgen => { let binaries = &["wasm-bindgen", "wasm-bindgen-test-runner"]; match cache.download(install_permitted, "wasm-bindgen", binaries, &url)? { - Some(download) => Ok(download), + Some(download) => Ok(Status::Found(download)), None => bail!("wasm-bindgen v{} is not installed!", version), } } Tool::CargoGenerate => { let binaries = &["cargo-generate"]; match cache.download(install_permitted, "cargo-generate", binaries, &url)? { - Some(download) => Ok(download), + Some(download) => Ok(Status::Found(download)), None => bail!("cargo-generate v{} is not installed!", version), } } + Tool::WasmOpt => { + let binaries = &["wasm-opt"]; + match cache.download(install_permitted, "wasm-opt", binaries, &url)? { + Some(download) => Ok(Status::Found(download)), + // TODO(ag_dubs): why is this different? i forget... + None => Ok(Status::CannotInstall), + } + } } } @@ -132,7 +163,10 @@ pub fn download_prebuilt( /// available for our host platform. fn prebuilt_url(tool: &Tool, version: &str) -> Result { let target = if target::LINUX && target::x86_64 { - "x86_64-unknown-linux-musl" + match tool { + Tool::WasmOpt => "x86-linux", + _ => "x86_64-unknown-linux-musl", + } } else if target::MACOS && target::x86_64 { "x86_64-apple-darwin" } else if target::WINDOWS && target::x86_64 { @@ -155,6 +189,13 @@ fn prebuilt_url(tool: &Tool, version: &str) -> Result { Krate::new(&Tool::CargoGenerate)?.max_version, target )) + }, + Tool::WasmOpt => { + Ok(format!( + "https://github.com/WebAssembly/binaryen/releases/download/{vers}/binaryen-{vers}-{target}.tar.gz", + vers = "version_78", + target = target, + )) } } } @@ -166,7 +207,7 @@ pub fn cargo_install( cache: &Cache, version: &str, install_permitted: bool, -) -> Result { +) -> Result { debug!( "Attempting to use a `cargo install`ed version of `{}={}`", tool, version, @@ -181,11 +222,12 @@ pub fn cargo_install( version, destination.display() ); - return Ok(Download::at(&destination)); + let download = Download::at(&destination); + return Ok(Status::Found(download)); } if !install_permitted { - bail!("{} v{} is not installed!", tool, version) + return Ok(Status::CannotInstall); } // Run `cargo install` to a temporary location to handle ctrl-c gracefully @@ -210,10 +252,6 @@ pub fn cargo_install( .arg("--root") .arg(&tmp); - if PBAR.quiet() { - cmd.arg("--quiet"); - } - let context = format!("Installing {} with cargo", tool); child::run(cmd, "cargo install").context(context)?; @@ -221,12 +259,13 @@ pub fn cargo_install( // just want them in `$root/*` directly (which matches how the tarballs are // laid out, and where the rest of our code expects them to be). So we do a // little renaming here. - let binaries = match tool { - Tool::WasmBindgen => vec!["wasm-bindgen", "wasm-bindgen-test-runner"], - Tool::CargoGenerate => vec!["cargo-genrate"], + let binaries: Result, failure::Error> = match tool { + Tool::WasmBindgen => Ok(vec!["wasm-bindgen", "wasm-bindgen-test-runner"]), + Tool::CargoGenerate => Ok(vec!["cargo-genrate"]), + Tool::WasmOpt => bail!("Cannot install wasm-opt with cargo."), }; - for b in binaries.iter().cloned() { + for b in binaries?.iter().cloned() { let from = tmp .join("bin") .join(b) @@ -245,5 +284,6 @@ pub fn cargo_install( // Finally, move the `tmp` directory into our binary cache. fs::rename(&tmp, &destination)?; - Ok(Download::at(&destination)) + let download = Download::at(&destination); + Ok(Status::Found(download)) } diff --git a/src/install/tool.rs b/src/install/tool.rs index c13f1921..4a174ea3 100644 --- a/src/install/tool.rs +++ b/src/install/tool.rs @@ -6,13 +6,17 @@ pub enum Tool { CargoGenerate, /// wasm-bindgen CLI tools WasmBindgen, + /// wasm-opt CLI tool + WasmOpt, } impl fmt::Display for Tool { fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { - match self { - Tool::CargoGenerate => write!(f, "cargo-generate"), - Tool::WasmBindgen => write!(f, "wasm-bindgen"), - } + let s = match self { + Tool::CargoGenerate => "cargo-generate", + Tool::WasmBindgen => "wasm-bindgen", + Tool::WasmOpt => "wasm-opt", + }; + write!(f, "{}", s) } } diff --git a/src/wasm_opt.rs b/src/wasm_opt.rs index c76818d2..19f33c3a 100644 --- a/src/wasm_opt.rs +++ b/src/wasm_opt.rs @@ -1,12 +1,10 @@ //! Support for downloading and executing `wasm-opt` use crate::child; -use crate::emoji; -use crate::target; +use crate::install::{self, Tool}; use crate::PBAR; use binary_install::Cache; -use log::debug; -use std::path::{Path, PathBuf}; +use std::path::Path; use std::process::Command; /// Execute `wasm-opt` over wasm binaries found in `out_dir`, downloading if @@ -18,17 +16,18 @@ pub fn run( install_permitted: bool, ) -> Result<(), failure::Error> { let wasm_opt = match find_wasm_opt(cache, install_permitted)? { - WasmOpt::Found(path) => path, - WasmOpt::CannotInstall => { + install::Status::Found(path) => path, + install::Status::CannotInstall => { PBAR.info("Skipping wasm-opt as no downloading was requested"); return Ok(()); } - WasmOpt::PlatformNotSupported => { + 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(&Tool::WasmOpt.to_string())?; PBAR.info("Optimizing wasm binaries with `wasm-opt`..."); for file in out_dir.read_dir()? { @@ -39,7 +38,7 @@ pub fn run( } let tmp = path.with_extension("wasm-opt.wasm"); - let mut cmd = Command::new(&wasm_opt); + let mut cmd = Command::new(&wasm_opt_path); cmd.arg(&path).arg("-o").arg(&tmp).args(args); child::run(cmd, "wasm-opt")?; std::fs::rename(&tmp, &path)?; @@ -48,16 +47,6 @@ pub fn run( Ok(()) } -/// Possible results of `find_wasm_opt` -pub enum WasmOpt { - /// Couldn't install wasm-opt because downloads are forbidden - CannotInstall, - /// The current platform doesn't support precompiled binaries - PlatformNotSupported, - /// We found `wasm-opt` at the specified path - Found(PathBuf), -} - /// Attempts to find `wasm-opt` in `PATH` locally, or failing that downloads a /// precompiled binary. /// @@ -65,44 +54,15 @@ pub enum WasmOpt { /// Returns `None` if a binary wasn't found in `PATH` and this platform doesn't /// have precompiled binaries. Returns an error if we failed to download the /// binary. -pub fn find_wasm_opt(cache: &Cache, install_permitted: bool) -> Result { - // First attempt to look up in PATH. If found assume it works. - if let Ok(path) = which::which("wasm-opt") { - debug!("found wasm-opt at {:?}", path); - return Ok(WasmOpt::Found(path)); - } - - // ... and if that fails download a precompiled version. - let target = if target::LINUX && target::x86_64 { - "x86_64-linux" - } else if target::MACOS && target::x86_64 { - "x86_64-apple-darwin" - } else if target::WINDOWS && target::x86_64 { - "x86_64-windows" - } else { - return Ok(WasmOpt::PlatformNotSupported); - }; - let url = format!( - "https://github.com/WebAssembly/binaryen/releases/download/{vers}/binaryen-{vers}-{target}.tar.gz", - vers = "version_78", - target = target, - ); - - let download = |permit_install| cache.download(permit_install, "wasm-opt", &["wasm-opt"], &url); - - let dl = match download(false)? { - Some(dl) => dl, - None if !install_permitted => return Ok(WasmOpt::CannotInstall), - None => { - let msg = format!("{}Installing wasm-opt...", emoji::DOWN_ARROW); - PBAR.info(&msg); - - match download(install_permitted)? { - Some(dl) => dl, - None => return Ok(WasmOpt::CannotInstall), - } - } - }; - - Ok(WasmOpt::Found(dl.binary("wasm-opt")?)) +pub fn find_wasm_opt( + cache: &Cache, + install_permitted: bool, +) -> Result { + let version = "version_78"; + Ok(install::download_prebuilt( + &install::Tool::WasmOpt, + cache, + version, + install_permitted, + )?) } diff --git a/tests/all/download.rs b/tests/all/download.rs index ca92877d..3f5dec14 100644 --- a/tests/all/download.rs +++ b/tests/all/download.rs @@ -11,9 +11,14 @@ use wasm_pack::install::{self, Tool}; fn can_download_prebuilt_wasm_bindgen() { let dir = tempfile::TempDir::new().unwrap(); let cache = Cache::at(dir.path()); - let dl = install::download_prebuilt(&Tool::WasmBindgen, &cache, "0.2.37", true).unwrap(); - assert!(dl.binary("wasm-bindgen").unwrap().is_file()); - assert!(dl.binary("wasm-bindgen-test-runner").unwrap().is_file()) + if let install::Status::Found(dl) = + install::download_prebuilt(&Tool::WasmBindgen, &cache, "0.2.37", true).unwrap() + { + assert!(dl.binary("wasm-bindgen").unwrap().is_file()); + assert!(dl.binary("wasm-bindgen-test-runner").unwrap().is_file()) + } else { + assert!(false, "Download failed") + } } #[test] @@ -45,6 +50,11 @@ fn downloading_prebuilt_wasm_bindgen_handles_http_errors() { fn can_download_prebuilt_cargo_generate() { let dir = tempfile::TempDir::new().unwrap(); let cache = Cache::at(dir.path()); - let dl = install::download_prebuilt(&Tool::CargoGenerate, &cache, "latest", true).unwrap(); - assert!(dl.binary("cargo-generate").unwrap().is_file()); + if let install::Status::Found(dl) = + install::download_prebuilt(&Tool::CargoGenerate, &cache, "latest", true).unwrap() + { + assert!(dl.binary("cargo-generate").unwrap().is_file()); + } else { + assert!(false, "Download Failed"); + } } diff --git a/tests/all/utils/fixture.rs b/tests/all/utils/fixture.rs index 5ba41957..6c7c1c48 100644 --- a/tests/all/utils/fixture.rs +++ b/tests/all/utils/fixture.rs @@ -238,7 +238,11 @@ impl Fixture { INSTALL_WASM_BINDGEN.call_once(|| { download().unwrap(); }); - download().unwrap().binary("wasm-bindgen").unwrap() + if let install::Status::Found(dl) = download().unwrap() { + dl.binary("wasm-bindgen").unwrap() + } else { + panic!("Download failed") + } } pub fn install_wasm_opt(&self) { @@ -273,7 +277,11 @@ impl Fixture { INSTALL_CARGO_GENERATE.call_once(|| { download().unwrap(); }); - download().unwrap().binary("cargo-generate").unwrap() + if let install::Status::Found(dl) = download().unwrap() { + dl.binary("cargo-generate").unwrap() + } else { + panic!("Download failed") + } } /// Download `geckodriver` and return its path. From 649bfabd26b9d913b9a41d9f17796d699a58ba1a Mon Sep 17 00:00:00 2001 From: Ashley Williams Date: Wed, 22 Jan 2020 18:47:28 -0600 Subject: [PATCH 2/3] feat(wasm-opt): bump to version_90 --- src/install/mod.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/install/mod.rs b/src/install/mod.rs index 79c8919d..086c7506 100644 --- a/src/install/mod.rs +++ b/src/install/mod.rs @@ -193,7 +193,7 @@ fn prebuilt_url(tool: &Tool, version: &str) -> Result { Tool::WasmOpt => { Ok(format!( "https://github.com/WebAssembly/binaryen/releases/download/{vers}/binaryen-{vers}-{target}.tar.gz", - vers = "version_78", + vers = "version_90", target = target, )) } From f0f058098da8854e158290dbab5ad390465947d3 Mon Sep 17 00:00:00 2001 From: Ashley Williams Date: Fri, 7 Feb 2020 10:05:24 -0600 Subject: [PATCH 3/3] feat(install): wasm-opt binaries add new conditional target logic --- src/install/mod.rs | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-) diff --git a/src/install/mod.rs b/src/install/mod.rs index 086c7506..7235cc78 100644 --- a/src/install/mod.rs +++ b/src/install/mod.rs @@ -167,10 +167,23 @@ fn prebuilt_url(tool: &Tool, version: &str) -> Result { Tool::WasmOpt => "x86-linux", _ => "x86_64-unknown-linux-musl", } + } else if target::LINUX && target::x86 { + match tool { + Tool::WasmOpt => "x86-linux", + _ => bail!("Unrecognized target!"), + } } else if target::MACOS && target::x86_64 { "x86_64-apple-darwin" } else if target::WINDOWS && target::x86_64 { - "x86_64-pc-windows-msvc" + match tool { + Tool::WasmOpt => "x86-windows", + _ => "x86_64-pc-windows-msvc", + } + } else if target::WINDOWS && target::x86 { + match tool { + Tool::WasmOpt => "x86-windows", + _ => bail!("Unrecognized target!"), + } } else { bail!("Unrecognized target!") };