Fix wasm-bindgen if lib is renamed via lib.name

This commit fixes an issue where if a library is renamed via the `name`
key in the `[lib]` section of the manifest then `wasm-pack` would try to
generate bindings for an noexistent wasm-file, generating an error.

The fix was to internally use `cargo_metadata` more aggressively and
move around where this data is generated. This ended up refactoring a
few locations, but this should also bring improved error messages for
`cargo metadata` as well as caching the resulting data more aggressively
to avoid recalculating it too much.

Closes rustwasm#339

src/bindgen.rs

@@ -1,10 +1,10 @@
 //! Functionality related to installing and running `wasm-bindgen`.
 
 use binaries::{Cache, Download};
-use cargo_metadata;
 use child;
 use emoji;
 use failure::{self, ResultExt};
+use manifest::CrateData;
 use progressbar::Step;
 use slog::Logger;
 use std::fs;
@@ -140,10 +140,9 @@ pub fn cargo_install_wasm_bindgen(
 /// Run the `wasm-bindgen` CLI to generate bindings for the current crate's
 /// `.wasm`.
 pub fn wasm_bindgen_build(
-    path: &Path,
+    data: &CrateData,
     bindgen: &Download,
     out_dir: &Path,
-    name: &str,
     disable_dts: bool,
     target: &str,
     debug: bool,
@@ -153,21 +152,16 @@ pub fn wasm_bindgen_build(
     let msg = format!("{}Running WASM-bindgen...", emoji::RUNNER);
     PBAR.step(step, &msg);
 
-    let binary_name = name.replace("-", "_");
     let release_or_debug = if debug { "debug" } else { "release" };
 
     let out_dir = out_dir.to_str().unwrap();
 
-    let manifest = path.join("Cargo.toml");
-    let target_path = cargo_metadata::metadata(Some(&manifest))
-        .unwrap()
-        .target_directory;
-    let mut wasm_path = PathBuf::from(&target_path)
+    let wasm_path = data
+        .target_directory()
         .join("wasm32-unknown-unknown")
         .join(release_or_debug)
-        .join(binary_name);
-    wasm_path.set_extension("wasm");
-    let wasm_path = wasm_path.display().to_string();
+        .join(data.crate_name())
+        .with_extension("wasm");
 
     let dts_arg = if disable_dts {
         "--no-typescript"
@@ -181,8 +175,7 @@ pub fn wasm_bindgen_build(
     };
     let bindgen_path = bindgen.binary("wasm-bindgen");
     let mut cmd = Command::new(bindgen_path);
-    cmd.current_dir(path)
-        .arg(&wasm_path)
+    cmd.arg(&wasm_path)
         .arg("--out-dir")
         .arg(out_dir)
         .arg(dts_arg)

src/command/build.rs

@@ -21,14 +21,13 @@ use PBAR;
 #[allow(missing_docs)]
 pub struct Build {
     pub crate_path: PathBuf,
-    pub crate_data: manifest::CargoManifest,
+    pub crate_data: manifest::CrateData,
     pub scope: Option<String>,
     pub disable_dts: bool,
     pub target: String,
     pub debug: bool,
     pub mode: BuildMode,
     // build_config: Option<BuildConfig>,
-    pub crate_name: String,
     pub out_dir: PathBuf,
     pub bindgen: Option<Download>,
     pub cache: Cache,
@@ -105,8 +104,7 @@ 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_data = manifest::CargoManifest::read(&crate_path)?;
-        let crate_name = crate_data.get_crate_name().to_string();
+        let crate_data = manifest::CrateData::new(&crate_path)?;
         let out_dir = crate_path.join(PathBuf::from(build_opts.out_dir));
         // let build_config = manifest::xxx(&crate_path).xxx();
         Ok(Build {
@@ -118,7 +116,6 @@ impl Build {
             debug: build_opts.debug,
             mode: build_opts.mode,
             // build_config,
-            crate_name,
             out_dir,
             bindgen: None,
             cache: Cache::new()?,
@@ -281,7 +278,7 @@ impl Build {
         log: &Logger,
     ) -> Result<(), failure::Error> {
         info!(&log, "Identifying wasm-bindgen dependency...");
-        let lockfile = Lockfile::new(&self.crate_path)?;
+        let lockfile = Lockfile::new(&self.crate_data)?;
         let bindgen_version = lockfile.require_wasm_bindgen()?;
         info!(&log, "Installing wasm-bindgen-cli...");
         let install_permitted = match self.mode {
@@ -298,25 +295,15 @@ impl Build {
         )?;
         self.bindgen = Some(bindgen);
         info!(&log, "Installing wasm-bindgen-cli was successful.");
-
-        info!(&log, "Getting the crate name from the manifest...");
-        self.crate_name = self.crate_data.get_crate_name().to_string();
-        info!(
-            &log,
-            "Got crate name {:#?} from the manifest at {:#?}.",
-            &self.crate_name,
-            &self.crate_path.join("Cargo.toml")
-        );
         Ok(())
     }
 
     fn step_run_wasm_bindgen(&mut self, step: &Step, log: &Logger) -> Result<(), failure::Error> {
         info!(&log, "Building the wasm bindings...");
         bindgen::wasm_bindgen_build(
-            &self.crate_path,
+            &self.crate_data,
             self.bindgen.as_ref().unwrap(),
             &self.out_dir,
-            &self.crate_name,
             self.disable_dts,
             &self.target,
             self.debug,

src/command/test.rs

@@ -82,7 +82,7 @@ pub struct TestOptions {
 /// A configured `wasm-pack test` command.
 pub struct Test {
     crate_path: PathBuf,
-    crate_data: manifest::CargoManifest,
+    crate_data: manifest::CrateData,
     cache: Cache,
     node: bool,
     mode: BuildMode,
@@ -117,7 +117,7 @@ impl Test {
         } = test_opts;
 
         let crate_path = set_crate_path(path)?;
-        let crate_data = manifest::CargoManifest::read(&crate_path)?;
+        let crate_data = manifest::CrateData::new(&crate_path)?;
         let any_browser = chrome || firefox || safari;
 
         if !node && !any_browser {
@@ -274,7 +274,7 @@ impl Test {
         log: &Logger,
     ) -> Result<(), failure::Error> {
         info!(&log, "Identifying wasm-bindgen dependency...");
-        let lockfile = Lockfile::new(&self.crate_path)?;
+        let lockfile = Lockfile::new(&self.crate_data)?;
         let bindgen_version = lockfile.require_wasm_bindgen()?;
 
         // Unlike `wasm-bindgen` and `wasm-bindgen-cli`, `wasm-bindgen-test`

src/lockfile.rs

@@ -1,11 +1,11 @@
 //! Reading Cargo.lock lock file.
 
 use std::fs;
-use std::path::{Path, PathBuf};
+use std::path::PathBuf;
 
-use cargo_metadata;
 use console::style;
-use error::Error;
+use failure::{Error, ResultExt};
+use manifest::CrateData;
 use toml;
 
 /// This struct represents the contents of `Cargo.lock`.
@@ -23,10 +23,13 @@ struct Package {
 
 impl Lockfile {
     /// Read the `Cargo.lock` file for the crate at the given path.
-    pub fn new(crate_path: &Path) -> Result<Lockfile, failure::Error> {
-        let lock_path = get_lockfile_path(crate_path)?;
-        let lockfile = fs::read_to_string(lock_path)?;
-        toml::from_str(&lockfile).map_err(|err| Error::from(err).into())
+    pub fn new(crate_data: &CrateData) -> Result<Lockfile, Error> {
+        let lock_path = get_lockfile_path(crate_data)?;
+        let lockfile = fs::read_to_string(&lock_path)
+            .with_context(|_| format!("failed to read: {}", lock_path.display()))?;
+        let lockfile = toml::from_str(&lockfile)
+            .with_context(|_| format!("failed to parse: {}", lock_path.display()))?;
+        Ok(lockfile)
     }
 
     /// Get the version of `wasm-bindgen` dependency used in the `Cargo.lock`.
@@ -36,15 +39,14 @@ impl Lockfile {
 
     /// Like `wasm_bindgen_version`, except it returns an error instead of
     /// `None`.
-    pub fn require_wasm_bindgen(&self) -> Result<&str, failure::Error> {
+    pub fn require_wasm_bindgen(&self) -> Result<&str, Error> {
         self.wasm_bindgen_version().ok_or_else(|| {
-            let message = format!(
+            format_err!(
                 "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 }.into()
+            )
         })
     }
 
@@ -63,22 +65,12 @@ impl Lockfile {
 
 /// Given the path to the crate that we are buliding, return a `PathBuf`
 /// containing the location of the lock file, by finding the workspace root.
-fn get_lockfile_path(crate_path: &Path) -> Result<PathBuf, failure::Error> {
-    // Identify the crate's root directory, or return an error.
-    let manifest = crate_path.join("Cargo.toml");
-    let crate_root = cargo_metadata::metadata(Some(&manifest))
-        .map_err(|_| Error::CrateConfig {
-            message: String::from("Error while processing crate metadata"),
-        })?
-        .workspace_root;
+fn get_lockfile_path(crate_data: &CrateData) -> Result<PathBuf, Error> {
     // Check that a lock file can be found in the directory. Return an error
     // if it cannot, otherwise return the path buffer.
-    let lockfile_path = Path::new(&crate_root).join("Cargo.lock");
+    let lockfile_path = crate_data.workspace_root().join("Cargo.lock");
     if !lockfile_path.is_file() {
-        Err(Error::CrateConfig {
-            message: format!("Could not find lockfile at {:?}", lockfile_path),
-        }
-        .into())
+        bail!("Could not find lockfile at {:?}", lockfile_path)
     } else {
         Ok(lockfile_path)
     }