Execute cargo metadata at most once

This can be somewhat expensive, so make sure we don't have to chew
through too much!

Author: alexcrichton

src/command/build.rs

@@ -6,7 +6,7 @@ use command::utils::{create_pkg_dir, set_crate_path};
 use emoji;
 use error::Error;
 use indicatif::HumanDuration;
-use lockfile;
+use lockfile::Lockfile;
 use manifest;
 use progressbar::Step;
 use readme;
@@ -240,7 +240,8 @@ impl Build {
 
     fn step_install_wasm_bindgen(&mut self, step: &Step, log: &Logger) -> Result<(), Error> {
         info!(&log, "Identifying wasm-bindgen dependency...");
-        let bindgen_version = lockfile::get_wasm_bindgen_version(&self.crate_path)?;
+        let lockfile = Lockfile::new(&self.crate_path)?;
+        let bindgen_version = lockfile.require_wasm_bindgen()?;
         info!(&log, "Installing wasm-bindgen-cli...");
         let install_permitted = match self.mode {
             BuildMode::Normal => true,

src/command/test.rs

@@ -4,10 +4,11 @@ use super::build::BuildMode;
 use bindgen;
 use build;
 use command::utils::set_crate_path;
+use console::style;
 use emoji;
 use error::Error;
 use indicatif::HumanDuration;
-use lockfile;
+use lockfile::Lockfile;
 use manifest;
 use progressbar::Step;
 use slog::Logger;
@@ -239,14 +240,23 @@ impl Test {
 
     fn step_install_wasm_bindgen(&mut self, step: &Step, log: &Logger) -> Result<(), Error> {
         info!(&log, "Identifying wasm-bindgen dependency...");
-        let bindgen_version = lockfile::get_wasm_bindgen_version(&self.crate_path)?;
+        let lockfile = Lockfile::new(&self.crate_path)?;
+        let bindgen_version = lockfile.require_wasm_bindgen()?;
 
         // Unlike `wasm-bindgen` and `wasm-bindgen-cli`, `wasm-bindgen-test`
         // will work with any semver compatible `wasm-bindgen-cli`, so just make
         // sure that it is depended upon, so we can run tests on
         // `wasm32-unkown-unknown`. Don't enforce that it is the same version as
         // `wasm-bindgen`.
-        let _bindgen_test_version = lockfile::get_wasm_bindgen_test_version(&self.crate_path)?;
+        if lockfile.wasm_bindgen_test_version().is_none() {
+            let message = format!(
+                "Ensure that you have \"{}\" as a dependency in your Cargo.toml file:\n\
+                 [dev-dependencies]\n\
+                 wasm-bindgen-test = \"0.2\"",
+                style("wasm-bindgen").bold().dim(),
+            );
+            return Err(Error::CrateConfig { message })
+        }
 
         let install_permitted = match self.mode {
             BuildMode::Normal => {

src/lockfile.rs

@@ -1,7 +1,6 @@
 //! Reading Cargo.lock lock file.
 
-use std::fs::File;
-use std::io::Read;
+use std::fs;
 use std::path::{Path, PathBuf};
 
 use cargo_metadata;
@@ -11,7 +10,7 @@ use toml;
 
 /// This struct represents the contents of `Cargo.lock`.
 #[derive(Clone, Debug, Deserialize)]
-struct Lockfile {
+pub struct Lockfile {
     package: Vec<Package>,
 }
 
@@ -23,50 +22,43 @@ struct Package {
 }
 
 impl Lockfile {
-    fn get_package_version(&self, package: &str) -> Option<String> {
-        self.package
-            .iter()
-            .find(|p| p.name == package)
-            .map(|p| p.version.clone())
+    /// Read the `Cargo.lock` file for the crate at the given path.
+    pub fn new(crate_path: &Path) -> Result<Lockfile, Error> {
+        let lock_path = get_lockfile_path(crate_path)?;
+        let lockfile = fs::read_to_string(lock_path)?;
+        toml::from_str(&lockfile).map_err(Error::from)
     }
-}
 
-/// Get the version of `wasm-bindgen` dependency used in the `Cargo.lock`.
-pub fn get_wasm_bindgen_version(path: &Path) -> Result<String, Error> {
-    let lockfile = read_cargo_lock(&path)?;
-    lockfile.get_package_version("wasm-bindgen").ok_or_else(|| {
-        let message = format!(
-            "Ensure that you have \"{}\" as a dependency in your Cargo.toml file:\n\
-             [dependencies]\n\
-             wasm-bindgen = \"0.2\"",
-            style("wasm-bindgen").bold().dim(),
-        );
-        Error::CrateConfig { message }
-    })
-}
+    /// Get the version of `wasm-bindgen` dependency used in the `Cargo.lock`.
+    pub fn wasm_bindgen_version(&self) -> Option<&str> {
+        self.get_package_version("wasm-bindgen")
+    }
 
-/// Get the version of `wasm-bindgen` dependency used in the `Cargo.lock`.
-pub fn get_wasm_bindgen_test_version(path: &Path) -> Result<String, Error> {
-    let lockfile = read_cargo_lock(&path)?;
-    lockfile
-        .get_package_version("wasm-bindgen-test")
-        .ok_or_else(|| {
+    /// Like `wasm_bindgen_version`, except it returns an error instead of
+    /// `None`.
+    pub fn require_wasm_bindgen(&self) -> Result<&str, Error> {
+        self.wasm_bindgen_version().ok_or_else(|| {
             let message = format!(
                 "Ensure that you have \"{}\" as a dependency in your Cargo.toml file:\n\
-                 [dev-dependencies]\n\
-                 wasm-bindgen-test = \"0.2\"",
+                 [dependencies]\n\
+                 wasm-bindgen = \"0.2\"",
                 style("wasm-bindgen").bold().dim(),
             );
             Error::CrateConfig { message }
         })
-}
+    }
+
+    /// Get the version of `wasm-bindgen` dependency used in the `Cargo.lock`.
+    pub fn wasm_bindgen_test_version(&self) -> Option<&str> {
+        self.get_package_version("wasm-bindgen-test")
+    }
 
-/// Read the `Cargo.lock` file for the crate at the given path.
-fn read_cargo_lock(crate_path: &Path) -> Result<Lockfile, Error> {
-    let lock_path = get_lockfile_path(crate_path)?;
-    let mut lockfile = String::new();
-    File::open(lock_path)?.read_to_string(&mut lockfile)?;
-    toml::from_str(&lockfile).map_err(Error::from)
+    fn get_package_version(&self, package: &str) -> Option<&str> {
+        self.package
+            .iter()
+            .find(|p| p.name == package)
+            .map(|p| &p.version[..])
+    }
 }
 
 /// Given the path to the crate that we are buliding, return a `PathBuf`

tests/all/lockfile.rs

@@ -1,23 +1,25 @@
 use utils::fixture;
-use wasm_pack::lockfile;
+use wasm_pack::lockfile::Lockfile;
 
 #[test]
 fn it_gets_wasm_bindgen_version() {
     let fixture = fixture::js_hello_world();
     fixture.cargo_check();
+    let lock = Lockfile::new(&fixture.path).unwrap();
     assert_eq!(
-        lockfile::get_wasm_bindgen_version(&fixture.path).unwrap(),
-        "0.2.21"
+        lock.wasm_bindgen_version(),
+        Some("0.2.21"),
     );
 }
 
 #[test]
 fn it_gets_wasm_bindgen_test_version() {
     let fixture = fixture::wbg_test_node();
     fixture.cargo_check();
+    let lock = Lockfile::new(&fixture.path).unwrap();
     assert_eq!(
-        lockfile::get_wasm_bindgen_test_version(&fixture.path).unwrap(),
-        "0.2.21"
+        lock.wasm_bindgen_test_version(),
+        Some("0.2.21"),
     );
 }
 
@@ -59,9 +61,10 @@ fn it_gets_wasm_bindgen_version_in_crate_inside_workspace() {
             "#,
         );
     fixture.cargo_check();
+    let lock = Lockfile::new(&fixture.path.join("blah")).unwrap();
     assert_eq!(
-        lockfile::get_wasm_bindgen_version(&fixture.path.join("blah")).unwrap(),
-        "0.2.21"
+        lock.wasm_bindgen_version(),
+        Some("0.2.21"),
     );
 }
 
@@ -124,8 +127,9 @@ fn it_gets_wasm_bindgen_version_from_dependencies() {
             "#,
         );
     fixture.cargo_check();
+    let lock = Lockfile::new(&fixture.path.join("parent")).unwrap();
     assert_eq!(
-        lockfile::get_wasm_bindgen_version(&fixture.path.join("parent")).unwrap(),
-        "0.2.21"
+        lock.wasm_bindgen_version(),
+        Some("0.2.21"),
     );
 }

tests/all/utils/fixture.rs

@@ -8,7 +8,7 @@ use std::sync::{Once, ONCE_INIT};
 use std::thread;
 use wasm_pack;
 
-use tempfile;
+use tempfile::TempDir;
 
 fn hard_link_or_copy<P1: AsRef<Path>, P2: AsRef<Path>>(from: P1, to: P2) -> io::Result<()> {
     let from = from.as_ref();
@@ -21,7 +21,7 @@ pub struct Fixture {
     // NB: we wrap the fixture's tempdir in a `ManuallyDrop` so that if a test
     // fails, its directory isn't deleted, and we have a chance to manually
     // inspect its state and figure out what is going on.
-    pub dir: ManuallyDrop<tempfile::TempDir>,
+    pub dir: ManuallyDrop<TempDir>,
     pub path: PathBuf,
 }
 
@@ -31,18 +31,20 @@ impl Fixture {
         // Make sure that all fixtures end up sharing a target dir, and we don't
         // recompile wasm-bindgen and friends many times over.
         static SET_TARGET_DIR: Once = ONCE_INIT;
+        let target_dir = Path::new(env!("CARGO_MANIFEST_DIR")).join("target");
         SET_TARGET_DIR.call_once(|| {
-            env::set_var(
-                "CARGO_TARGET_DIR",
-                Path::new(env!("CARGO_MANIFEST_DIR")).join("target"),
-            );
+            env::set_var("CARGO_TARGET_DIR", &target_dir);
         });
 
-        let dir =
-            ManuallyDrop::new(tempfile::tempdir().expect("should create temporary directory OK"));
+        let root = target_dir.join("t");
+        fs::create_dir_all(&root).unwrap();
+        let dir = TempDir::new_in(&root).unwrap();
         let path = dir.path().join("wasm-pack");
         eprintln!("Created fixture at {}", path.display());
-        Fixture { dir, path }
+        Fixture {
+            dir: ManuallyDrop::new(dir),
+            path,
+        }
     }
 
     /// Create a file within this fixture.