Merge pull request #504 from fitzgen/more-logging-and-assertions

More logging and assertions

binary-install/Cargo.toml

@@ -14,6 +14,7 @@ dirs = "1.0.4"
 failure = "0.1.2"
 flate2 = "1.0.2"
 hex = "0.3"
+is_executable = "0.1.2"
 siphasher = "0.2.3"
 tar = "0.4.16"
 zip = "0.5.0"

binary-install/src/lib.rs

@@ -6,6 +6,7 @@ extern crate failure;
 extern crate dirs;
 extern crate flate2;
 extern crate hex;
+extern crate is_executable;
 extern crate siphasher;
 extern crate tar;
 extern crate zip;
@@ -223,13 +224,22 @@ impl Download {
     }
 
     /// Returns the path to the binary `name` within this download
-    pub fn binary(&self, name: &str) -> PathBuf {
+    pub fn binary(&self, name: &str) -> Result<PathBuf, Error> {
+        use is_executable::IsExecutable;
+
         let ret = self
             .root
             .join(name)
             .with_extension(env::consts::EXE_EXTENSION);
-        assert!(ret.exists(), "binary {} doesn't exist", ret.display());
-        return ret;
+
+        if !ret.is_file() {
+            bail!("{} binary does not exist", ret.display());
+        }
+        if !ret.is_executable() {
+            bail!("{} is not executable", ret.display());
+        }
+
+        Ok(ret)
     }
 }
 

src/bindgen.rs

@@ -9,6 +9,7 @@ use log::debug;
 use log::{info, warn};
 use manifest::CrateData;
 use progressbar::Step;
+use std::env;
 use std::fs;
 use std::path::{Path, PathBuf};
 use std::process::Command;
@@ -101,9 +102,19 @@ pub fn cargo_install_wasm_bindgen(
     version: &str,
     install_permitted: bool,
 ) -> Result<Download, failure::Error> {
+    debug!(
+        "Attempting to use a `cargo install`ed version of `wasm-bindgen={}`",
+        version
+    );
+
     let dirname = format!("wasm-bindgen-cargo-install-{}", version);
     let destination = cache.join(dirname.as_ref());
     if destination.exists() {
+        debug!(
+            "`cargo install`ed `wasm-bindgen={}` already exists at {}",
+            version,
+            destination.display()
+        );
         return Ok(Download::at(&destination));
     }
 
@@ -115,7 +126,12 @@ pub fn cargo_install_wasm_bindgen(
     // and ensure we don't accidentally use stale files in the future
     let tmp = cache.join(format!(".{}", dirname).as_ref());
     drop(fs::remove_dir_all(&tmp));
-    fs::create_dir_all(&tmp)?;
+    debug!(
+        "cargo installing wasm-bindgen to tempdir: {}",
+        tmp.display()
+    );
+    fs::create_dir_all(&tmp)
+        .context("failed to create temp dir for `cargo install wasm-bindgen`")?;
 
     let mut cmd = Command::new("cargo");
     cmd.arg("install")
@@ -128,7 +144,28 @@ pub fn cargo_install_wasm_bindgen(
 
     child::run(cmd, "cargo install").context("Installing wasm-bindgen with cargo")?;
 
+    // `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.
+    for f in ["wasm-bindgen", "wasm-bindgen-test-runner"].iter().cloned() {
+        let from = tmp
+            .join("bin")
+            .join(f)
+            .with_extension(env::consts::EXE_EXTENSION);
+        let to = tmp.join(from.file_name().unwrap());
+        fs::rename(&from, &to).with_context(|_| {
+            format!(
+                "failed to move {} to {} for `cargo install`ed `wasm-bindgen`",
+                from.display(),
+                to.display()
+            )
+        })?;
+    }
+
+    // Finally, move the `tmp` directory into our binary cache.
     fs::rename(&tmp, &destination)?;
+
     Ok(Download::at(&destination))
 }
 
@@ -170,7 +207,7 @@ pub fn wasm_bindgen_build(
         "no-modules" => "--no-modules",
         _ => "--browser",
     };
-    let bindgen_path = bindgen.binary("wasm-bindgen");
+    let bindgen_path = bindgen.binary("wasm-bindgen")?;
     let mut cmd = Command::new(bindgen_path);
     cmd.arg(&wasm_path)
         .arg("--out-dir")

src/command/test.rs

@@ -288,7 +288,7 @@ impl Test {
         let dl =
             bindgen::install_wasm_bindgen(&self.cache, &bindgen_version, install_permitted, step)?;
 
-        self.test_runner_path = Some(dl.binary("wasm-bindgen-test-runner"));
+        self.test_runner_path = Some(dl.binary("wasm-bindgen-test-runner")?);
 
         info!("Getting wasm-bindgen-cli was successful.");
         Ok(())

src/test/webdriver.rs

@@ -47,7 +47,7 @@ pub fn install_chromedriver(
         &["chromedriver"],
         &url,
     )? {
-        Some(dl) => Ok(dl.binary("chromedriver")),
+        Some(dl) => Ok(dl.binary("chromedriver")?),
         None => bail!(
             "No cached `chromedriver` binary found, and could not find a global \
              `chromedriver` on the `$PATH`. Not installing `chromedriver` because of noinstall \
@@ -97,7 +97,7 @@ pub fn install_geckodriver(
         ext,
     );
     match cache.download(installation_allowed, "geckodriver", &["geckodriver"], &url)? {
-        Some(dl) => Ok(dl.binary("geckodriver")),
+        Some(dl) => Ok(dl.binary("geckodriver")?),
         None => bail!(
             "No cached `geckodriver` binary found, and could not find a global `geckodriver` \
              on the `$PATH`. Not installing `geckodriver` because of noinstall mode."

tests/all/bindgen.rs

@@ -12,8 +12,8 @@ fn can_download_prebuilt_wasm_bindgen() {
     let dir = tempfile::TempDir::new().unwrap();
     let cache = Cache::at(dir.path());
     let dl = bindgen::download_prebuilt_wasm_bindgen(&cache, "0.2.21", true).unwrap();
-    assert!(dl.binary("wasm-bindgen").is_file());
-    assert!(dl.binary("wasm-bindgen-test-runner").is_file())
+    assert!(dl.binary("wasm-bindgen").unwrap().is_file());
+    assert!(dl.binary("wasm-bindgen-test-runner").unwrap().is_file())
 }
 
 #[test]

tests/all/utils/fixture.rs

@@ -185,7 +185,7 @@ impl Fixture {
         INSTALL_WASM_BINDGEN.call_once(|| {
             download().unwrap();
         });
-        download().unwrap().binary("wasm-bindgen")
+        download().unwrap().binary("wasm-bindgen").unwrap()
     }
 
     /// Download `geckodriver` and return its path.