Merge pull request #687 from rustwasm/refactor-opt

refactor(tool): wasm-opt is a tool variant

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<String>,
     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<bool, failure::Error> {
 }
 
 /// Check if the `wasm-bindgen` dependency is locally satisfied for the --target flag
-fn supports_dash_dash_target(cli_path: &PathBuf) -> Result<bool, failure::Error> {
+fn supports_dash_dash_target(cli_path: PathBuf) -> Result<bool, failure::Error> {
     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<String, failure::Error> {
-    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())

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<String>,
-    pub bindgen: Option<Download>,
+    pub bindgen: Option<install::Status>,
     pub cache: Cache,
     pub extra_options: Vec<String>,
 }
@@ -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,

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(())

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);

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<Download, failure::Error> {
+) -> Result<Status, failure::Error> {
     // 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<Download, failure::Error> {
+) -> Result<Status, failure::Error> {
     let url = match prebuilt_url(tool, version) {
         Ok(url) => url,
         Err(e) => bail!(
@@ -114,29 +137,53 @@ 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),
+            }
+        }
     }
 }
 
 /// Returns the URL of a precompiled version of wasm-bindgen, if we have one
 /// available for our host platform.
 fn prebuilt_url(tool: &Tool, version: &str) -> Result<String, failure::Error> {
     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::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!")
     };
@@ -155,6 +202,13 @@ fn prebuilt_url(tool: &Tool, version: &str) -> Result<String, failure::Error> {
                 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_90",
+        target = target,
+            ))
         }
     }
 }
@@ -166,7 +220,7 @@ pub fn cargo_install(
     cache: &Cache,
     version: &str,
     install_permitted: bool,
-) -> Result<Download, failure::Error> {
+) -> Result<Status, failure::Error> {
     debug!(
         "Attempting to use a `cargo install`ed version of `{}={}`",
         tool, version,
@@ -181,11 +235,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,23 +265,20 @@ 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)?;
 
     // `cargo install` will put the installed binaries in `$root/bin/*`, but we
     // 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<Vec<&str>, 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 +297,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))
 }

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)
     }
 }