Merge pull request #431 from fitzgen/stop-reading-cargo-toml-many-times

Stop reading cargo toml many times

src/command/build.rs

@@ -21,6 +21,7 @@ use PBAR;
 #[allow(missing_docs)]
 pub struct Build {
     pub crate_path: PathBuf,
+    pub crate_data: manifest::CargoManifest,
     pub scope: Option<String>,
     pub disable_dts: bool,
     pub target: String,
@@ -104,11 +105,13 @@ impl Build {
     /// Construct a build command from the given options.
     pub fn try_from_opts(build_opts: BuildOptions) -> Result<Self, failure::Error> {
         let crate_path = set_crate_path(build_opts.path)?;
-        let crate_name = manifest::get_crate_name(&crate_path)?;
+        let crate_data = manifest::CargoManifest::read(&crate_path)?;
+        let crate_name = crate_data.get_crate_name().to_string();
         let out_dir = crate_path.join(PathBuf::from(build_opts.out_dir));
         // let build_config = manifest::xxx(&crate_path).xxx();
         Ok(Build {
             crate_path,
+            crate_data,
             scope: build_opts.scope,
             disable_dts: build_opts.disable_dts,
             target: build_opts.target,
@@ -213,7 +216,7 @@ impl Build {
 
     fn step_check_crate_config(&mut self, step: &Step, log: &Logger) -> Result<(), failure::Error> {
         info!(&log, "Checking crate configuration...");
-        manifest::check_crate_config(&self.crate_path, step)?;
+        self.crate_data.check_crate_config(step)?;
         info!(&log, "Crate is correctly configured.");
         Ok(())
     }
@@ -250,8 +253,7 @@ impl Build {
 
     fn step_create_json(&mut self, step: &Step, log: &Logger) -> Result<(), failure::Error> {
         info!(&log, "Writing a package.json...");
-        manifest::write_package_json(
-            &self.crate_path,
+        self.crate_data.write_package_json(
             &self.out_dir,
             &self.scope,
             self.disable_dts,
@@ -298,7 +300,7 @@ impl Build {
         info!(&log, "Installing wasm-bindgen-cli was successful.");
 
         info!(&log, "Getting the crate name from the manifest...");
-        self.crate_name = manifest::get_crate_name(&self.crate_path)?;
+        self.crate_name = self.crate_data.get_crate_name().to_string();
         info!(
             &log,
             "Got crate name {:#?} from the manifest at {:#?}.",

src/command/test.rs

@@ -82,6 +82,7 @@ pub struct TestOptions {
 /// A configured `wasm-pack test` command.
 pub struct Test {
     crate_path: PathBuf,
+    crate_data: manifest::CargoManifest,
     cache: Cache,
     node: bool,
     mode: BuildMode,
@@ -116,13 +117,7 @@ impl Test {
         } = test_opts;
 
         let crate_path = set_crate_path(path)?;
-
-        // let geckodriver = get_web_driver("geckodriver", test_opts.geckodriver, test_opts.firefox)?;
-        // let chromedriver =
-        //     get_web_driver("chromedriver", test_opts.chromedriver, test_opts.chrome)?;
-        // let safaridriver =
-        //     get_web_driver("safaridriver", test_opts.safaridriver, test_opts.safari)?;
-
+        let crate_data = manifest::CargoManifest::read(&crate_path)?;
         let any_browser = chrome || firefox || safari;
 
         if !node && !any_browser {
@@ -143,6 +138,7 @@ impl Test {
         Ok(Test {
             cache: Cache::new()?,
             crate_path,
+            crate_data,
             node,
             mode,
             chrome,
@@ -248,7 +244,7 @@ impl Test {
 
     fn step_check_crate_config(&mut self, step: &Step, log: &Logger) -> Result<(), failure::Error> {
         info!(log, "Checking crate configuration...");
-        manifest::check_crate_config(&self.crate_path, step)?;
+        self.crate_data.check_crate_config(step)?;
         info!(log, "Crate is correctly configured.");
         Ok(())
     }

src/manifest/mod.rs

@@ -18,8 +18,9 @@ use serde_json;
 use toml;
 use PBAR;
 
-#[derive(Debug, Deserialize)]
-struct CargoManifest {
+/// A parsed `Cargo.toml` manifest.
+#[derive(Clone, Debug, Deserialize)]
+pub struct CargoManifest {
     package: CargoPackage,
     dependencies: Option<HashMap<String, CargoDependency>>,
     #[serde(rename = "dev-dependencies")]
@@ -27,7 +28,7 @@ struct CargoManifest {
     lib: Option<CargoLib>,
 }
 
-#[derive(Debug, Deserialize)]
+#[derive(Clone, Debug, Deserialize)]
 struct CargoPackage {
     name: String,
     authors: Vec<String>,
@@ -59,42 +60,99 @@ impl CargoPackage {
     }
 }
 
-#[derive(Debug, Deserialize)]
+#[derive(Clone, Debug, Deserialize)]
 #[serde(untagged)]
 enum CargoDependency {
     Simple(String),
     Detailed(DetailedCargoDependency),
 }
 
-#[derive(Debug, Deserialize)]
+#[derive(Clone, Debug, Deserialize)]
 struct DetailedCargoDependency {
     version: Option<String>,
 }
 
-#[derive(Debug, Deserialize)]
+#[derive(Clone, Debug, Deserialize)]
 struct CargoLib {
     #[serde(rename = "crate-type")]
     crate_type: Option<Vec<String>>,
 }
 
-fn read_cargo_toml(path: &Path) -> Result<CargoManifest, failure::Error> {
-    let manifest_path = path.join("Cargo.toml");
-    if !manifest_path.is_file() {
-        return Err(Error::crate_config(&format!(
-            "Crate directory is missing a `Cargo.toml` file; is `{}` the wrong directory?",
-            path.display()
-        ))
-        .into());
+impl CargoManifest {
+    /// Read the `Cargo.toml` inside the crate at the given `crate_path`.
+    pub fn read(crate_path: &Path) -> Result<CargoManifest, failure::Error> {
+        let manifest_path = crate_path.join("Cargo.toml");
+        if !manifest_path.is_file() {
+            return Err(Error::crate_config(&format!(
+                "Crate directory is missing a `Cargo.toml` file; is `{}` the wrong directory?",
+                crate_path.display()
+            ))
+            .into());
+        }
+        let mut cargo_file = File::open(manifest_path)?;
+        let mut cargo_contents = String::new();
+        cargo_file.read_to_string(&mut cargo_contents)?;
+
+        let manifest: CargoManifest = toml::from_str(&cargo_contents)?;
+        Ok(manifest)
     }
-    let mut cargo_file = File::open(manifest_path)?;
-    let mut cargo_contents = String::new();
-    cargo_file.read_to_string(&mut cargo_contents)?;
 
-    let manifest: CargoManifest = toml::from_str(&cargo_contents)?;
-    Ok(manifest)
-}
+    /// Generate a package.json file inside in `./pkg`.
+    pub fn write_package_json(
+        &self,
+        out_dir: &Path,
+        scope: &Option<String>,
+        disable_dts: bool,
+        target: &str,
+        step: &Step,
+    ) -> Result<(), failure::Error> {
+        let msg = format!("{}Writing a package.json...", emoji::MEMO);
+
+        PBAR.step(step, &msg);
+        let pkg_file_path = out_dir.join("package.json");
+        let mut pkg_file = File::create(pkg_file_path)?;
+        let npm_data = if target == "nodejs" {
+            self.clone().into_commonjs(scope, disable_dts)
+        } else if target == "no-modules" {
+            self.clone().into_nomodules(scope, disable_dts)
+        } else {
+            self.clone().into_esmodules(scope, disable_dts)
+        };
+
+        let npm_json = serde_json::to_string_pretty(&npm_data)?;
+        pkg_file.write_all(npm_json.as_bytes())?;
+        Ok(())
+    }
+
+    /// Get the crate name for the crate at the given path.
+    pub fn get_crate_name(&self) -> &str {
+        &self.package.name
+    }
+
+    /// Check that the crate the given path is properly configured.
+    pub fn check_crate_config(&self, step: &Step) -> Result<(), failure::Error> {
+        let msg = format!("{}Checking crate configuration...", emoji::WRENCH);
+        PBAR.step(&step, &msg);
+        self.check_crate_type()?;
+        Ok(())
+    }
+
+    fn check_crate_type(&self) -> Result<(), failure::Error> {
+        if self.lib.as_ref().map_or(false, |lib| {
+            lib.crate_type
+                .as_ref()
+                .map_or(false, |types| types.iter().any(|s| s == "cdylib"))
+        }) {
+            return Ok(());
+        }
+        Err(Error::crate_config(
+            "crate-type must be cdylib to compile to wasm32-unknown-unknown. Add the following to your \
+             Cargo.toml file:\n\n\
+             [lib]\n\
+             crate-type = [\"cdylib\", \"rlib\"]"
+        ).into())
+    }
 
-impl CargoManifest {
     fn into_commonjs(mut self, scope: &Option<String>, disable_dts: bool) -> NpmPackage {
         let filename = self.package.name.replace("-", "_");
         let wasm_file = format!("{}_bg.wasm", filename);
@@ -207,59 +265,3 @@ impl CargoManifest {
         })
     }
 }
-
-/// Generate a package.json file inside in `./pkg`.
-pub fn write_package_json(
-    path: &Path,
-    out_dir: &Path,
-    scope: &Option<String>,
-    disable_dts: bool,
-    target: &str,
-    step: &Step,
-) -> Result<(), failure::Error> {
-    let msg = format!("{}Writing a package.json...", emoji::MEMO);
-
-    PBAR.step(step, &msg);
-    let pkg_file_path = out_dir.join("package.json");
-    let mut pkg_file = File::create(pkg_file_path)?;
-    let crate_data = read_cargo_toml(path)?;
-    let npm_data = if target == "nodejs" {
-        crate_data.into_commonjs(scope, disable_dts)
-    } else if target == "no-modules" {
-        crate_data.into_nomodules(scope, disable_dts)
-    } else {
-        crate_data.into_esmodules(scope, disable_dts)
-    };
-
-    let npm_json = serde_json::to_string_pretty(&npm_data)?;
-    pkg_file.write_all(npm_json.as_bytes())?;
-    Ok(())
-}
-
-/// Get the crate name for the crate at the given path.
-pub fn get_crate_name(path: &Path) -> Result<String, failure::Error> {
-    Ok(read_cargo_toml(path)?.package.name)
-}
-
-/// Check that the crate the given path is properly configured.
-pub fn check_crate_config(path: &Path, step: &Step) -> Result<(), failure::Error> {
-    let msg = format!("{}Checking crate configuration...", emoji::WRENCH);
-    PBAR.step(&step, &msg);
-    check_crate_type(path)?;
-    Ok(())
-}
-
-fn check_crate_type(path: &Path) -> Result<(), failure::Error> {
-    if read_cargo_toml(path)?.lib.map_or(false, |lib| {
-        lib.crate_type
-            .map_or(false, |types| types.iter().any(|s| s == "cdylib"))
-    }) {
-        return Ok(());
-    }
-    Err(Error::crate_config(
-      "crate-type must be cdylib to compile to wasm32-unknown-unknown. Add the following to your \
-       Cargo.toml file:\n\n\
-       [lib]\n\
-       crate-type = [\"cdylib\", \"rlib\"]"
-    ).into())
-}

tests/all/manifest.rs

@@ -8,18 +8,16 @@ use wasm_pack::{self, manifest};
 #[test]
 fn it_gets_the_crate_name_default_path() {
     let path = &PathBuf::from(".");
-    assert!(manifest::get_crate_name(path).is_ok());
-    assert_eq!(manifest::get_crate_name(path).unwrap(), "wasm-pack");
+    let crate_data = manifest::CargoManifest::read(&path).unwrap();
+    let name = crate_data.get_crate_name();
+    assert_eq!(name, "wasm-pack");
 }
 
 #[test]
 fn it_gets_the_crate_name_provided_path() {
     let fixture = fixture::js_hello_world();
-    assert!(manifest::get_crate_name(&fixture.path).is_ok());
-    assert_eq!(
-        manifest::get_crate_name(&fixture.path).unwrap(),
-        "js-hello-world"
-    );
+    let crate_data = manifest::CargoManifest::read(&fixture.path).unwrap();
+    assert_eq!(crate_data.get_crate_name(), "js-hello-world");
 }
 
 #[test]
@@ -27,8 +25,9 @@ fn it_checks_has_cdylib_default_path() {
     let fixture = fixture::no_cdylib();
     // Ensure that there is a `Cargo.lock`.
     fixture.cargo_check();
+    let crate_data = manifest::CargoManifest::read(&fixture.path).unwrap();
     let step = wasm_pack::progressbar::Step::new(1);
-    assert!(manifest::check_crate_config(&fixture.path, &step).is_err());
+    assert!(crate_data.check_crate_config(&step).is_err());
 }
 
 #[test]
@@ -36,15 +35,17 @@ fn it_checks_has_cdylib_provided_path() {
     let fixture = fixture::js_hello_world();
     // Ensure that there is a `Cargo.lock`.
     fixture.cargo_check();
+    let crate_data = manifest::CargoManifest::read(&fixture.path).unwrap();
     let step = wasm_pack::progressbar::Step::new(1);
-    assert!(manifest::check_crate_config(&fixture.path, &step).is_ok());
+    assert!(crate_data.check_crate_config(&step).is_ok());
 }
 
 #[test]
 fn it_checks_has_cdylib_wrong_crate_type() {
     let fixture = fixture::bad_cargo_toml();
+    let crate_data = manifest::CargoManifest::read(&fixture.path).unwrap();
     let step = wasm_pack::progressbar::Step::new(1);
-    assert!(manifest::check_crate_config(&fixture.path, &step).is_err());
+    assert!(crate_data.check_crate_config(&step).is_err());
 }
 
 #[test]
@@ -52,17 +53,21 @@ fn it_recognizes_a_map_during_depcheck() {
     let fixture = fixture::serde_feature();
     // Ensure that there is a `Cargo.lock`.
     fixture.cargo_check();
+    let crate_data = manifest::CargoManifest::read(&fixture.path).unwrap();
     let step = wasm_pack::progressbar::Step::new(1);
-    assert!(manifest::check_crate_config(&fixture.path, &step).is_ok());
+    assert!(crate_data.check_crate_config(&step).is_ok());
 }
 
 #[test]
 fn it_creates_a_package_json_default_path() {
     let fixture = fixture::js_hello_world();
     let out_dir = fixture.path.join("pkg");
+    let crate_data = manifest::CargoManifest::read(&fixture.path).unwrap();
     let step = wasm_pack::progressbar::Step::new(1);
     wasm_pack::command::utils::create_pkg_dir(&out_dir, &step).unwrap();
-    assert!(manifest::write_package_json(&fixture.path, &out_dir, &None, false, "", &step).is_ok());
+    assert!(crate_data
+        .write_package_json(&out_dir, &None, false, "", &step)
+        .is_ok());
     let package_json_path = &fixture.path.join("pkg").join("package.json");
     assert!(fs::metadata(package_json_path).is_ok());
     assert!(utils::manifest::read_package_json(&fixture.path, &out_dir).is_ok());
@@ -93,9 +98,12 @@ fn it_creates_a_package_json_default_path() {
 fn it_creates_a_package_json_provided_path() {
     let fixture = fixture::js_hello_world();
     let out_dir = fixture.path.join("pkg");
+    let crate_data = manifest::CargoManifest::read(&fixture.path).unwrap();
     let step = wasm_pack::progressbar::Step::new(1);
     wasm_pack::command::utils::create_pkg_dir(&out_dir, &step).unwrap();
-    assert!(manifest::write_package_json(&fixture.path, &out_dir, &None, false, "", &step).is_ok());
+    assert!(crate_data
+        .write_package_json(&out_dir, &None, false, "", &step)
+        .is_ok());
     let package_json_path = &fixture.path.join("pkg").join("package.json");
     assert!(fs::metadata(package_json_path).is_ok());
     utils::manifest::read_package_json(&fixture.path, &out_dir).unwrap();
@@ -119,17 +127,12 @@ fn it_creates_a_package_json_provided_path() {
 fn it_creates_a_package_json_provided_path_with_scope() {
     let fixture = fixture::js_hello_world();
     let out_dir = fixture.path.join("pkg");
+    let crate_data = manifest::CargoManifest::read(&fixture.path).unwrap();
     let step = wasm_pack::progressbar::Step::new(1);
     wasm_pack::command::utils::create_pkg_dir(&out_dir, &step).unwrap();
-    assert!(manifest::write_package_json(
-        &fixture.path,
-        &out_dir,
-        &Some("test".to_string()),
-        false,
-        "",
-        &step
-    )
-    .is_ok());
+    assert!(crate_data
+        .write_package_json(&out_dir, &Some("test".to_string()), false, "", &step)
+        .is_ok());
     let package_json_path = &fixture.path.join("pkg").join("package.json");
     assert!(fs::metadata(package_json_path).is_ok());
     assert!(utils::manifest::read_package_json(&fixture.path, &out_dir).is_ok());
@@ -153,12 +156,12 @@ fn it_creates_a_package_json_provided_path_with_scope() {
 fn it_creates_a_pkg_json_with_correct_files_on_node() {
     let fixture = fixture::js_hello_world();
     let out_dir = fixture.path.join("pkg");
+    let crate_data = manifest::CargoManifest::read(&fixture.path).unwrap();
     let step = wasm_pack::progressbar::Step::new(1);
     wasm_pack::command::utils::create_pkg_dir(&out_dir, &step).unwrap();
-    assert!(
-        manifest::write_package_json(&fixture.path, &out_dir, &None, false, "nodejs", &step)
-            .is_ok()
-    );
+    assert!(crate_data
+        .write_package_json(&out_dir, &None, false, "nodejs", &step)
+        .is_ok());
     let package_json_path = &out_dir.join("package.json");
     assert!(fs::metadata(package_json_path).is_ok());
     utils::manifest::read_package_json(&fixture.path, &out_dir).unwrap();
@@ -188,17 +191,12 @@ fn it_creates_a_pkg_json_with_correct_files_on_node() {
 fn it_creates_a_pkg_json_with_correct_files_on_nomodules() {
     let fixture = fixture::js_hello_world();
     let out_dir = fixture.path.join("pkg");
+    let crate_data = manifest::CargoManifest::read(&fixture.path).unwrap();
     let step = wasm_pack::progressbar::Step::new(1);
     wasm_pack::command::utils::create_pkg_dir(&out_dir, &step).unwrap();
-    assert!(manifest::write_package_json(
-        &fixture.path,
-        &out_dir,
-        &None,
-        false,
-        "no-modules",
-        &step
-    )
-    .is_ok());
+    assert!(crate_data
+        .write_package_json(&out_dir, &None, false, "no-modules", &step)
+        .is_ok());
     let package_json_path = &out_dir.join("package.json");
     assert!(fs::metadata(package_json_path).is_ok());
     utils::manifest::read_package_json(&fixture.path, &out_dir).unwrap();
@@ -228,9 +226,12 @@ fn it_creates_a_pkg_json_with_correct_files_on_nomodules() {
 fn it_creates_a_pkg_json_in_out_dir() {
     let fixture = fixture::js_hello_world();
     let out_dir = fixture.path.join("./custom/out");
+    let crate_data = manifest::CargoManifest::read(&fixture.path).unwrap();
     let step = wasm_pack::progressbar::Step::new(1);
     wasm_pack::command::utils::create_pkg_dir(&out_dir, &step).unwrap();
-    assert!(manifest::write_package_json(&fixture.path, &out_dir, &None, false, "", &step).is_ok());
+    assert!(crate_data
+        .write_package_json(&out_dir, &None, false, "", &step)
+        .is_ok());
 
     let package_json_path = &fixture.path.join(&out_dir).join("package.json");
     assert!(fs::metadata(package_json_path).is_ok());
@@ -241,9 +242,12 @@ fn it_creates_a_pkg_json_in_out_dir() {
 fn it_creates_a_package_json_with_correct_keys_when_types_are_skipped() {
     let fixture = fixture::js_hello_world();
     let out_dir = fixture.path.join("pkg");
+    let crate_data = manifest::CargoManifest::read(&fixture.path).unwrap();
     let step = wasm_pack::progressbar::Step::new(1);
     wasm_pack::command::utils::create_pkg_dir(&out_dir, &step).unwrap();
-    assert!(manifest::write_package_json(&fixture.path, &out_dir, &None, true, "", &step).is_ok());
+    assert!(crate_data
+        .write_package_json(&out_dir, &None, true, "", &step)
+        .is_ok());
     let package_json_path = &out_dir.join("package.json");
     assert!(fs::metadata(package_json_path).is_ok());
     assert!(utils::manifest::read_package_json(&fixture.path, &out_dir).is_ok());
@@ -267,8 +271,9 @@ fn it_creates_a_package_json_with_correct_keys_when_types_are_skipped() {
 #[test]
 fn it_errors_when_wasm_bindgen_is_not_declared() {
     let fixture = fixture::bad_cargo_toml();
+    let crate_data = manifest::CargoManifest::read(&fixture.path).unwrap();
     let step = wasm_pack::progressbar::Step::new(1);
-    assert!(manifest::check_crate_config(&fixture.path, &step).is_err());
+    assert!(crate_data.check_crate_config(&step).is_err());
 }
 
 #[test]
@@ -276,6 +281,7 @@ fn it_does_not_error_when_wasm_bindgen_is_declared() {
     let fixture = fixture::js_hello_world();
     // Ensure that there is a `Cargo.lock`.
     fixture.cargo_check();
+    let crate_data = manifest::CargoManifest::read(&fixture.path).unwrap();
     let step = wasm_pack::progressbar::Step::new(1);
-    assert!(manifest::check_crate_config(&fixture.path, &step).is_ok());
+    assert!(crate_data.check_crate_config(&step).is_ok());
 }