From dd872113234062634b565d6a72d9b54903b5a6c1 Mon Sep 17 00:00:00 2001 From: data-pup Date: Thu, 30 Aug 2018 15:46:12 -0400 Subject: [PATCH 1/4] feat(lockfiles): Use `Cargo.lock` to identify wasm-bindgen versions This lets us leverage `cargo` for semver finding and then ensure that we get the exact same version of the CLI that cargo selected. It also lets us support fuzzy dependencies like "0.2" instead of exact dependencies like "0.2.21" again. --- Cargo.lock | 24 +++++++ Cargo.toml | 1 + src/command/build.rs | 3 +- src/command/test.rs | 23 +++++-- src/lib.rs | 2 + src/lockfile.rs | 91 ++++++++++++++++++++++++++ src/manifest/mod.rs | 117 +-------------------------------- tests/all/lockfile.rs | 131 +++++++++++++++++++++++++++++++++++++ tests/all/main.rs | 1 + tests/all/manifest.rs | 40 +++-------- tests/all/test.rs | 14 ++++ tests/all/utils/fixture.rs | 68 ++++++++++--------- 12 files changed, 332 insertions(+), 183 deletions(-) create mode 100644 src/lockfile.rs create mode 100644 tests/all/lockfile.rs diff --git a/Cargo.lock b/Cargo.lock index 0298da4..415be84 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -83,6 +83,18 @@ dependencies = [ "libc 0.2.43 (registry+https://github.com/rust-lang/crates.io-index)", ] +[[package]] +name = "cargo_metadata" +version = "0.6.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +dependencies = [ + "error-chain 0.12.0 (registry+https://github.com/rust-lang/crates.io-index)", + "semver 0.9.0 (registry+https://github.com/rust-lang/crates.io-index)", + "serde 1.0.79 (registry+https://github.com/rust-lang/crates.io-index)", + "serde_derive 1.0.79 (registry+https://github.com/rust-lang/crates.io-index)", + "serde_json 1.0.28 (registry+https://github.com/rust-lang/crates.io-index)", +] + [[package]] name = "cc" version = "1.0.25" @@ -189,6 +201,14 @@ dependencies = [ "winapi 0.3.5 (registry+https://github.com/rust-lang/crates.io-index)", ] +[[package]] +name = "error-chain" +version = "0.12.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +dependencies = [ + "backtrace 0.3.9 (registry+https://github.com/rust-lang/crates.io-index)", +] + [[package]] name = "failure" version = "0.1.2" @@ -634,6 +654,7 @@ version = "0.9.0" source = "registry+https://github.com/rust-lang/crates.io-index" dependencies = [ "semver-parser 0.7.0 (registry+https://github.com/rust-lang/crates.io-index)", + "serde 1.0.79 (registry+https://github.com/rust-lang/crates.io-index)", ] [[package]] @@ -941,6 +962,7 @@ name = "wasm-pack" version = "0.4.2" dependencies = [ "atty 0.2.11 (registry+https://github.com/rust-lang/crates.io-index)", + "cargo_metadata 0.6.0 (registry+https://github.com/rust-lang/crates.io-index)", "console 0.6.2 (registry+https://github.com/rust-lang/crates.io-index)", "curl 0.4.17 (registry+https://github.com/rust-lang/crates.io-index)", "failure 0.1.2 (registry+https://github.com/rust-lang/crates.io-index)", @@ -1042,6 +1064,7 @@ dependencies = [ "checksum byteorder 1.2.6 (registry+https://github.com/rust-lang/crates.io-index)" = "90492c5858dd7d2e78691cfb89f90d273a2800fc11d98f60786e5d87e2f83781" "checksum bzip2 0.3.3 (registry+https://github.com/rust-lang/crates.io-index)" = "42b7c3cbf0fa9c1b82308d57191728ca0256cb821220f4e2fd410a72ade26e3b" "checksum bzip2-sys 0.1.6 (registry+https://github.com/rust-lang/crates.io-index)" = "2c5162604199bbb17690ede847eaa6120a3f33d5ab4dcc8e7c25b16d849ae79b" +"checksum cargo_metadata 0.6.0 (registry+https://github.com/rust-lang/crates.io-index)" = "2d6809b327f87369e6f3651efd2c5a96c49847a3ed2559477ecba79014751ee1" "checksum cc 1.0.25 (registry+https://github.com/rust-lang/crates.io-index)" = "f159dfd43363c4d08055a07703eb7a3406b0dac4d0584d96965a3262db3c9d16" "checksum cfg-if 0.1.5 (registry+https://github.com/rust-lang/crates.io-index)" = "0c4e7bb64a8ebb0d856483e1e682ea3422f883c5f5615a90d51a2c82fe87fdd3" "checksum chrono 0.4.6 (registry+https://github.com/rust-lang/crates.io-index)" = "45912881121cb26fad7c38c17ba7daa18764771836b34fab7d3fbd93ed633878" @@ -1052,6 +1075,7 @@ dependencies = [ "checksum crc 1.8.1 (registry+https://github.com/rust-lang/crates.io-index)" = "d663548de7f5cca343f1e0a48d14dcfb0e9eb4e079ec58883b7251539fa10aeb" "checksum curl 0.4.17 (registry+https://github.com/rust-lang/crates.io-index)" = "c8172e96ecfb1a2bfe3843d9d7154099a15130cf4a2f658259c7aa9cc2b5d4ff" "checksum curl-sys 0.4.12 (registry+https://github.com/rust-lang/crates.io-index)" = "78800a6de442f65dab6ce26c6f369c14fc585686432bf4b77119d2d384216c31" +"checksum error-chain 0.12.0 (registry+https://github.com/rust-lang/crates.io-index)" = "07e791d3be96241c77c43846b665ef1384606da2cd2a48730abe606a12906e02" "checksum failure 0.1.2 (registry+https://github.com/rust-lang/crates.io-index)" = "7efb22686e4a466b1ec1a15c2898f91fa9cb340452496dca654032de20ff95b9" "checksum failure_derive 0.1.2 (registry+https://github.com/rust-lang/crates.io-index)" = "946d0e98a50d9831f5d589038d2ca7f8f455b1c21028c0db0e84116a12696426" "checksum filetime 0.2.1 (registry+https://github.com/rust-lang/crates.io-index)" = "da4b9849e77b13195302c174324b5ba73eec9b236b24c221a61000daefb95c5f" diff --git a/Cargo.toml b/Cargo.toml index 9a8f314..0077cf6 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -10,6 +10,7 @@ categories = ["wasm"] [dependencies] atty = "0.2.11" +cargo_metadata = "0.6.0" console = "0.6.1" curl = "0.4.13" failure = "0.1.2" diff --git a/src/command/build.rs b/src/command/build.rs index 461abb8..40b812c 100644 --- a/src/command/build.rs +++ b/src/command/build.rs @@ -6,6 +6,7 @@ use command::utils::{create_pkg_dir, set_crate_path}; use emoji; use error::Error; use indicatif::HumanDuration; +use lockfile; use manifest; use progressbar::Step; use readme; @@ -239,7 +240,7 @@ impl Build { fn step_install_wasm_bindgen(&mut self, step: &Step, log: &Logger) -> Result<(), Error> { info!(&log, "Identifying wasm-bindgen dependency..."); - let bindgen_version = manifest::get_wasm_bindgen_version(&self.crate_path)?; + let bindgen_version = lockfile::get_wasm_bindgen_version(&self.crate_path)?; info!(&log, "Installing wasm-bindgen-cli..."); let install_permitted = match self.mode { BuildMode::Normal => true, diff --git a/src/command/test.rs b/src/command/test.rs index 5bba1b9..6977045 100644 --- a/src/command/test.rs +++ b/src/command/test.rs @@ -7,6 +7,7 @@ use command::utils::set_crate_path; use emoji; use error::Error; use indicatif::HumanDuration; +use lockfile; use manifest; use progressbar::Step; use slog::Logger; @@ -238,12 +239,24 @@ impl Test { fn step_install_wasm_bindgen(&mut self, step: &Step, log: &Logger) -> Result<(), Error> { info!(&log, "Identifying wasm-bindgen dependency..."); - let bindgen_version = manifest::get_wasm_bindgen_version(&self.crate_path)?; - info!(&log, "Installing wasm-bindgen-cli..."); + let bindgen_version = lockfile::get_wasm_bindgen_version(&self.crate_path)?; + + // 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)?; let install_permitted = match self.mode { - BuildMode::Normal => true, - BuildMode::Noinstall => false, + BuildMode::Normal => { + info!(&log, "Ensuring wasm-bindgen-cli is installed..."); + true + } + BuildMode::Noinstall => { + info!(&log, "Searching for existing wasm-bindgen-cli install..."); + false + } }; bindgen::install_wasm_bindgen( @@ -257,7 +270,7 @@ impl Test { self.test_runner_path = Some(bindgen::wasm_bindgen_test_runner_path(log, &self.crate_path) .expect("if installing wasm-bindgen succeeded, then we should have wasm-bindgen-test-runner too")); - info!(&log, "Installing wasm-bindgen-cli was successful."); + info!(&log, "Getting wasm-bindgen-cli was successful."); Ok(()) } diff --git a/src/lib.rs b/src/lib.rs index 7db8931..bb3f2b1 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -2,6 +2,7 @@ #![deny(missing_docs)] +extern crate cargo_metadata; extern crate console; extern crate curl; #[macro_use] @@ -31,6 +32,7 @@ pub mod build; pub mod command; pub mod emoji; pub mod error; +pub mod lockfile; pub mod logger; pub mod manifest; pub mod npm; diff --git a/src/lockfile.rs b/src/lockfile.rs new file mode 100644 index 0000000..0540402 --- /dev/null +++ b/src/lockfile.rs @@ -0,0 +1,91 @@ +//! Reading Cargo.lock lock file. + +use std::fs::File; +use std::io::Read; +use std::path::{Path, PathBuf}; + +use cargo_metadata; +use console::style; +use error::Error; +use toml; + +/// This struct represents the contents of `Cargo.lock`. +#[derive(Clone, Debug, Deserialize)] +struct Lockfile { + package: Vec, +} + +/// This struct represents a single package entry in `Cargo.lock` +#[derive(Clone, Debug, Deserialize)] +struct Package { + name: String, + version: String, +} + +impl Lockfile { + fn get_package_version(&self, package: &str) -> Option { + self.package + .iter() + .find(|p| p.name == package) + .map(|p| p.version.clone()) + } +} + +/// Get the version of `wasm-bindgen` dependency used in the `Cargo.lock`. +pub fn get_wasm_bindgen_version(path: &Path) -> Result { + 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 get_wasm_bindgen_test_version(path: &Path) -> Result { + let lockfile = read_cargo_lock(&path)?; + lockfile + .get_package_version("wasm-bindgen-test") + .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\"", + style("wasm-bindgen").bold().dim(), + ); + Error::CrateConfig { message } + }) +} + +/// Read the `Cargo.lock` file for the crate at the given path. +fn read_cargo_lock(crate_path: &Path) -> Result { + 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) +} + +/// 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 { + // 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; + // 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"); + if !lockfile_path.is_file() { + Err(Error::CrateConfig { + message: format!("Could not find lockfile at {:?}", lockfile_path), + }) + } else { + Ok(lockfile_path) + } +} diff --git a/src/manifest/mod.rs b/src/manifest/mod.rs index f59baef..9244b11 100644 --- a/src/manifest/mod.rs +++ b/src/manifest/mod.rs @@ -27,31 +27,6 @@ struct CargoManifest { lib: Option, } -fn normalize_dependency_name(dep: &str) -> String { - dep.replace("-", "_") -} - -fn normalize_dependencies( - deps: HashMap, -) -> HashMap { - let mut new_deps = HashMap::with_capacity(deps.len()); - for (key, val) in deps { - new_deps.insert(normalize_dependency_name(&key), val); - } - new_deps -} - -impl CargoManifest { - fn normalize_dependencies(&mut self) { - if let Some(deps) = self.dependencies.take() { - self.dependencies = Some(normalize_dependencies(deps)); - } - if let Some(dev_deps) = self.dev_dependencies.take() { - self.dev_dependencies = Some(normalize_dependencies(dev_deps)); - } - } -} - #[derive(Debug, Deserialize)] struct CargoPackage { name: String, @@ -113,9 +88,7 @@ fn read_cargo_toml(path: &Path) -> Result { let mut cargo_contents = String::new(); cargo_file.read_to_string(&mut cargo_contents)?; - let mut manifest: CargoManifest = toml::from_str(&cargo_contents)?; - manifest.normalize_dependencies(); - + let manifest: CargoManifest = toml::from_str(&cargo_contents)?; Ok(manifest) } @@ -270,35 +243,10 @@ pub fn get_crate_name(path: &Path) -> Result { pub fn check_crate_config(path: &Path, step: &Step) -> Result<(), Error> { let msg = format!("{}Checking crate configuration...", emoji::WRENCH); PBAR.step(&step, &msg); - check_wasm_bindgen(path)?; - check_wasm_bindgen_test(path)?; check_crate_type(path)?; Ok(()) } -fn check_wasm_bindgen(path: &Path) -> Result<(), Error> { - get_wasm_bindgen_version(path)?; - Ok(()) -} - -fn check_wasm_bindgen_test(path: &Path) -> Result<(), Error> { - let expected_version = get_wasm_bindgen_version(path)?; - - // Only do the version check if `wasm-bindgen-test` is actually a - // dependency. Not every crate needs to have tests! - if let Ok(actual_version) = get_wasm_bindgen_test_version(path) { - if expected_version != actual_version { - return Error::crate_config(&format!( - "The `wasm-bindgen-test` dependency version ({}) must match \ - the `wasm-bindgen` dependency version ({}), but it does not.", - actual_version, expected_version - )); - } - } - - Ok(()) -} - fn check_crate_type(path: &Path) -> Result<(), Error> { if read_cargo_toml(path)?.lib.map_or(false, |lib| { lib.crate_type @@ -310,67 +258,6 @@ fn check_crate_type(path: &Path) -> Result<(), Error> { "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\"]" - ) -} - -fn get_dependency_version( - dependencies: Option<&HashMap>, - dependency: &str, - dependencies_section_name: &str, - version_suggestion: &str, -) -> Result { - if let Some(deps) = dependencies { - let dependency = normalize_dependency_name(dependency); - match deps.get(&dependency) { - Some(CargoDependency::Simple(version)) - | Some(CargoDependency::Detailed(DetailedCargoDependency { - version: Some(version), - })) => Ok(version.clone()), - Some(CargoDependency::Detailed(DetailedCargoDependency { version: None })) => { - let msg = format!( - "\"{}\" dependency is missing its version number", - style(&dependency).bold().dim() - ); - Err(Error::CrateConfig { message: msg }) - } - None => { - let message = format!( - "Ensure that you have \"{}\" as a dependency in your Cargo.toml file:\n\ - [{}]\n\ - {} = \"{}\"", - style(&dependency).bold().dim(), - dependencies_section_name, - dependency, - version_suggestion - ); - Err(Error::CrateConfig { message }) - } - } - } else { - let message = String::from("Could not find crate dependencies"); - Err(Error::CrateConfig { message }) - } -} - -/// Get the version of `wasm-bindgen` specified as a dependency. -pub fn get_wasm_bindgen_version(path: &Path) -> Result { - let toml = read_cargo_toml(path)?; - get_dependency_version( - toml.dependencies.as_ref(), - "wasm-bindgen", - "dependencies", - "0.2", - ) -} - -/// Get the version of `wasm-bindgen-test` specified as a dependency. -pub fn get_wasm_bindgen_test_version(path: &Path) -> Result { - let toml = read_cargo_toml(path)?; - get_dependency_version( - toml.dev_dependencies.as_ref(), - "wasm-bindgen-test", - "dev-dependencies", - "0.2", + crate-type = [\"cdylib\", \"rlib\"]" ) } diff --git a/tests/all/lockfile.rs b/tests/all/lockfile.rs new file mode 100644 index 0000000..b7ec668 --- /dev/null +++ b/tests/all/lockfile.rs @@ -0,0 +1,131 @@ +use utils::fixture; +use wasm_pack::lockfile; + +#[test] +fn it_gets_wasm_bindgen_version() { + let fixture = fixture::js_hello_world(); + fixture.cargo_check(); + assert_eq!( + lockfile::get_wasm_bindgen_version(&fixture.path).unwrap(), + "0.2.21" + ); +} + +#[test] +fn it_gets_wasm_bindgen_test_version() { + let fixture = fixture::wbg_test_node(); + fixture.cargo_check(); + assert_eq!( + lockfile::get_wasm_bindgen_test_version(&fixture.path).unwrap(), + "0.2.21" + ); +} + +#[test] +fn it_gets_wasm_bindgen_version_in_crate_inside_workspace() { + let fixture = fixture::Fixture::new(); + fixture + .file( + "Cargo.toml", + r#" + [workspace] + members = ["./blah"] + "#, + ).file( + "blah/Cargo.toml", + r#" + [package] + authors = ["The wasm-pack developers"] + description = "so awesome rust+wasm package" + license = "WTFPL" + name = "blah" + repository = "https://github.com/rustwasm/wasm-pack.git" + version = "0.1.0" + + [lib] + crate-type = ["cdylib"] + + [dependencies] + wasm-bindgen = "=0.2.21" + "#, + ).file( + "blah/src/lib.rs", + r#" + extern crate wasm_bindgen; + use wasm_bindgen::prelude::*; + + #[wasm_bindgen] + pub fn hello() -> u32 { 42 } + "#, + ); + fixture.cargo_check(); + assert_eq!( + lockfile::get_wasm_bindgen_version(&fixture.path.join("blah")).unwrap(), + "0.2.21" + ); +} + +#[test] +fn it_gets_wasm_bindgen_version_from_dependencies() { + let fixture = fixture::Fixture::new(); + fixture + .file( + "Cargo.toml", + r#" + [workspace] + members = ["./parent", "./child"] + "#, + ).file( + "child/Cargo.toml", + r#" + [package] + authors = ["The wasm-pack developers"] + description = "so awesome rust+wasm package" + license = "WTFPL" + name = "child" + repository = "https://github.com/rustwasm/wasm-pack.git" + version = "0.1.0" + + [lib] + crate-type = ["cdylib"] + + [dependencies] + wasm-bindgen = "=0.2.21" + "#, + ).file( + "child/src/lib.rs", + r#" + extern crate wasm_bindgen; + use wasm_bindgen::prelude::*; + + #[wasm_bindgen] + pub fn hello() -> u32 { 42 } + "#, + ).file( + "parent/Cargo.toml", + r#" + [package] + authors = ["The wasm-pack developers"] + description = "so awesome rust+wasm package" + license = "WTFPL" + name = "parent" + repository = "https://github.com/rustwasm/wasm-pack.git" + version = "0.1.0" + + [lib] + crate-type = ["cdylib"] + "#, + ).file( + "parent/src/lib.rs", + r#" + // Just re-export all of `child`. + extern crate child; + pub use child::*; + "#, + ); + fixture.cargo_check(); + assert_eq!( + lockfile::get_wasm_bindgen_version(&fixture.path.join("parent")).unwrap(), + "0.2.21" + ); +} diff --git a/tests/all/main.rs b/tests/all/main.rs index ebf6485..2d56b04 100644 --- a/tests/all/main.rs +++ b/tests/all/main.rs @@ -8,6 +8,7 @@ extern crate wasm_pack; mod bindgen; mod build; +mod lockfile; mod manifest; mod readme; mod test; diff --git a/tests/all/manifest.rs b/tests/all/manifest.rs index 8d84e35..7d6c8e8 100644 --- a/tests/all/manifest.rs +++ b/tests/all/manifest.rs @@ -25,6 +25,8 @@ fn it_gets_the_crate_name_provided_path() { #[test] fn it_checks_has_cdylib_default_path() { let fixture = fixture::no_cdylib(); + // Ensure that there is a `Cargo.lock`. + fixture.cargo_check(); let step = wasm_pack::progressbar::Step::new(1); assert!(manifest::check_crate_config(&fixture.path, &step).is_err()); } @@ -32,6 +34,8 @@ fn it_checks_has_cdylib_default_path() { #[test] fn it_checks_has_cdylib_provided_path() { let fixture = fixture::js_hello_world(); + // Ensure that there is a `Cargo.lock`. + fixture.cargo_check(); let step = wasm_pack::progressbar::Step::new(1); assert!(manifest::check_crate_config(&fixture.path, &step).is_ok()); } @@ -46,6 +50,8 @@ fn it_checks_has_cdylib_wrong_crate_type() { #[test] fn it_recognizes_a_map_during_depcheck() { let fixture = fixture::serde_feature(); + // Ensure that there is a `Cargo.lock`. + fixture.cargo_check(); let step = wasm_pack::progressbar::Step::new(1); assert!(manifest::check_crate_config(&fixture.path, &step).is_ok()); } @@ -264,38 +270,8 @@ fn it_errors_when_wasm_bindgen_is_not_declared() { #[test] 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 step = wasm_pack::progressbar::Step::new(1); assert!(manifest::check_crate_config(&fixture.path, &step).is_ok()); } - -#[test] -fn it_gets_wasm_bindgen_version() { - let fixture = fixture::js_hello_world(); - assert_eq!( - manifest::get_wasm_bindgen_version(&fixture.path).unwrap(), - "0.2.21" - ); -} - -#[test] -fn it_gets_wasm_bindgen_version_with_underscores() { - let fixture = fixture::with_underscores(); - assert_eq!( - manifest::get_wasm_bindgen_version(&fixture.path).unwrap(), - "0.2" - ); -} - -#[test] -fn the_wasm_bindgen_test_version_should_match_the_wasm_bindgen_version() { - let fixture = fixture::wbg_test_bad_versions(); - let step = wasm_pack::progressbar::Step::new(1); - let result = manifest::check_crate_config(&fixture.path, &step); - assert!(result.is_err()); - let msg = result.unwrap_err().to_string(); - println!("{}", msg); - assert!(msg.contains(&format!( - "The `wasm-bindgen-test` dependency version (=0.2.19) must match \ - the `wasm-bindgen` dependency version (=0.2.21), but it does not." - ))); -} diff --git a/tests/all/test.rs b/tests/all/test.rs index ba71d1c..63757b1 100644 --- a/tests/all/test.rs +++ b/tests/all/test.rs @@ -20,6 +20,20 @@ fn it_can_run_node_tests() { command::run_wasm_pack(cmd, &logger).expect("should run test command OK"); } +#[test] +fn it_can_run_tests_with_different_wbg_test_and_wbg_versions() { + let fixture = fixture::wbg_test_diff_versions(); + fixture.install_local_wasm_bindgen(); + let cmd = Command::Test(test::TestOptions { + path: Some(fixture.path.clone()), + node: true, + mode: build::BuildMode::Noinstall, + ..Default::default() + }); + let logger = logger::new(&cmd, 3).unwrap(); + command::run_wasm_pack(cmd, &logger).expect("should run test command OK"); +} + #[test] #[cfg(any( all(target_os = "linux", target_arch = "x86_64"), diff --git a/tests/all/utils/fixture.rs b/tests/all/utils/fixture.rs index 08f26a2..93e5a6d 100644 --- a/tests/all/utils/fixture.rs +++ b/tests/all/utils/fixture.rs @@ -3,6 +3,7 @@ use std::fs; use std::io; use std::mem::ManuallyDrop; use std::path::{Path, PathBuf}; +use std::process::{Command, Stdio}; use std::sync::{Once, ONCE_INIT}; use std::thread; use wasm_pack; @@ -40,6 +41,7 @@ impl Fixture { let dir = ManuallyDrop::new(tempfile::tempdir().expect("should create temporary directory OK")); let path = dir.path().join("wasm-pack"); + eprintln!("Created fixture at {}", path.display()); Fixture { dir, path } } @@ -236,6 +238,24 @@ impl Fixture { self } + + /// The `step_install_wasm_bindgen` and `step_run_wasm_bindgen` steps only + /// occur after the `step_build_wasm` step. In order to read the lockfile + /// in the test fixture's temporary directory, we should first build the + /// crate, targeting `wasm32-unknown-unknown`. + pub fn cargo_check(&self) -> &Self { + Command::new("cargo") + .current_dir(&self.path) + .arg("+nightly") + .arg("check") + .arg("--target") + .arg("wasm32-unknown-unknown") + .stdout(Stdio::null()) + .stderr(Stdio::null()) + .status() + .unwrap(); + self + } } impl Drop for Fixture { @@ -328,40 +348,50 @@ pub fn serde_feature() -> Fixture { fixture } -pub fn wbg_test_bad_versions() -> Fixture { +pub fn wbg_test_diff_versions() -> Fixture { let fixture = Fixture::new(); fixture .readme() - .hello_world_src_lib() .file( "Cargo.toml", r#" [package] - name = "wbg-test-node" + name = "wbg-test-diff-versions" version = "0.1.0" authors = ["The wasm-pack developers"] [lib] - crate-type = ["cdylib"] + crate-type = ["cdylib", "rlib"] [dependencies] # We depend on wasm-bindgen 0.2.21 wasm-bindgen = "=0.2.21" [dev-dependencies] - # And we depend on wasm-bindgen-test 0.2.19. But this should match the - # wasm-bindgen dependency! - wasm-bindgen-test = "=0.2.19" + # And we depend on wasm-bindgen-test 0.2.19. This should still + # work, and we should end up with `wasm-bindgen` at 0.2.21 and + # wasm-bindgen-test at 0.2.19, and everything should still work. + wasm-bindgen-test = "0.2.19" + "#, + ).file( + "src/lib.rs", + r#" + extern crate wasm_bindgen; + use wasm_bindgen::prelude::*; + + #[wasm_bindgen] + pub fn one() -> u32 { 1 } "#, ).file( "tests/node.rs", r#" + extern crate wbg_test_diff_versions; extern crate wasm_bindgen_test; use wasm_bindgen_test::*; #[wasm_bindgen_test] fn pass() { - assert_eq!(1, 1); + assert_eq!(wbg_test_diff_versions::one(), 1); } "#, ); @@ -432,25 +462,3 @@ pub fn wbg_test_node() -> Fixture { ); fixture } - -pub fn with_underscores() -> Fixture { - let fixture = Fixture::new(); - fixture.readme().hello_world_src_lib().file( - "Cargo.toml", - r#" - [package] - name = "with-underscores" - version = "0.1.0" - authors = ["The wasm-pack developers"] - - [lib] - crate-type = ["cdylib"] - - [dependencies] - # Cargo will normalize "wasm-bindgen" and "wasm_bindgen" and that shouldn't - # break wasm-pack. - wasm_bindgen = "0.2" - "#, - ); - fixture -} From 3d1f528fc9530bc75173eb8bdfc54fb55fcdde11 Mon Sep 17 00:00:00 2001 From: Alex Crichton Date: Mon, 24 Sep 2018 11:46:35 -0700 Subject: [PATCH 2/4] Make tests more future-proof with wasm-bindgen This way when future wasm-bindgen version are published the tests will continue working! --- src/manifest/mod.rs | 1 - tests/all/utils/fixture.rs | 8 ++++---- 2 files changed, 4 insertions(+), 5 deletions(-) diff --git a/src/manifest/mod.rs b/src/manifest/mod.rs index 9244b11..eb2744c 100644 --- a/src/manifest/mod.rs +++ b/src/manifest/mod.rs @@ -10,7 +10,6 @@ use std::path::Path; use self::npm::{ repository::Repository, CommonJSPackage, ESModulesPackage, NoModulesPackage, NpmPackage, }; -use console::style; use emoji; use error::Error; use progressbar::Step; diff --git a/tests/all/utils/fixture.rs b/tests/all/utils/fixture.rs index 93e5a6d..9b5e1d1 100644 --- a/tests/all/utils/fixture.rs +++ b/tests/all/utils/fixture.rs @@ -94,10 +94,10 @@ impl Fixture { crate-type = ["cdylib"] [dependencies] - wasm-bindgen = "0.2.21" + wasm-bindgen = "=0.2.21" [dev-dependencies] - wasm-bindgen-test = "0.2.21" + wasm-bindgen-test = "=0.2.21" "#, name ), @@ -312,10 +312,10 @@ pub fn no_cdylib() -> Fixture { # crate-type = ["cdylib"] [dependencies] - wasm-bindgen = "0.2.21" + wasm-bindgen = "=0.2.21" [dev-dependencies] - wasm-bindgen-test = "0.2.21" + wasm-bindgen-test = "=0.2.21" "#, ); fixture From 9b24dcb25979e56acdfde5b49acbecd84d9b74bf Mon Sep 17 00:00:00 2001 From: Alex Crichton Date: Mon, 24 Sep 2018 11:57:06 -0700 Subject: [PATCH 3/4] Execute `cargo metadata` at most once This can be somewhat expensive, so make sure we don't have to chew through too much! --- src/command/build.rs | 5 +-- src/command/test.rs | 16 +++++++-- src/lockfile.rs | 66 +++++++++++++++++--------------------- tests/all/lockfile.rs | 22 +++++++------ tests/all/utils/fixture.rs | 20 ++++++------ 5 files changed, 69 insertions(+), 60 deletions(-) diff --git a/src/command/build.rs b/src/command/build.rs index 40b812c..8a6016d 100644 --- a/src/command/build.rs +++ b/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, diff --git a/src/command/test.rs b/src/command/test.rs index 6977045..fc3afa2 100644 --- a/src/command/test.rs +++ b/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 => { diff --git a/src/lockfile.rs b/src/lockfile.rs index 0540402..2ed5110 100644 --- a/src/lockfile.rs +++ b/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, } @@ -23,50 +22,43 @@ struct Package { } impl Lockfile { - fn get_package_version(&self, package: &str) -> Option { - 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 { + 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 { - 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 { - 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 { - 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` diff --git a/tests/all/lockfile.rs b/tests/all/lockfile.rs index b7ec668..d8958eb 100644 --- a/tests/all/lockfile.rs +++ b/tests/all/lockfile.rs @@ -1,13 +1,14 @@ 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"), ); } @@ -15,9 +16,10 @@ fn it_gets_wasm_bindgen_version() { 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"), ); } diff --git a/tests/all/utils/fixture.rs b/tests/all/utils/fixture.rs index 9b5e1d1..a0fc509 100644 --- a/tests/all/utils/fixture.rs +++ b/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, P2: AsRef>(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, + pub dir: ManuallyDrop, 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. From 3856db20b95c1c803c4234d959e9d263ebfa1464 Mon Sep 17 00:00:00 2001 From: Ashley Williams Date: Mon, 24 Sep 2018 15:35:08 -0400 Subject: [PATCH 4/4] fix(style): appease cargo fmt --- src/command/test.rs | 2 +- tests/all/lockfile.rs | 20 ++++---------------- 2 files changed, 5 insertions(+), 17 deletions(-) diff --git a/src/command/test.rs b/src/command/test.rs index fc3afa2..ed803dc 100644 --- a/src/command/test.rs +++ b/src/command/test.rs @@ -255,7 +255,7 @@ impl Test { wasm-bindgen-test = \"0.2\"", style("wasm-bindgen").bold().dim(), ); - return Err(Error::CrateConfig { message }) + return Err(Error::CrateConfig { message }); } let install_permitted = match self.mode { diff --git a/tests/all/lockfile.rs b/tests/all/lockfile.rs index d8958eb..1ee6e3b 100644 --- a/tests/all/lockfile.rs +++ b/tests/all/lockfile.rs @@ -6,10 +6,7 @@ fn it_gets_wasm_bindgen_version() { let fixture = fixture::js_hello_world(); fixture.cargo_check(); let lock = Lockfile::new(&fixture.path).unwrap(); - assert_eq!( - lock.wasm_bindgen_version(), - Some("0.2.21"), - ); + assert_eq!(lock.wasm_bindgen_version(), Some("0.2.21"),); } #[test] @@ -17,10 +14,7 @@ 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!( - lock.wasm_bindgen_test_version(), - Some("0.2.21"), - ); + assert_eq!(lock.wasm_bindgen_test_version(), Some("0.2.21"),); } #[test] @@ -62,10 +56,7 @@ fn it_gets_wasm_bindgen_version_in_crate_inside_workspace() { ); fixture.cargo_check(); let lock = Lockfile::new(&fixture.path.join("blah")).unwrap(); - assert_eq!( - lock.wasm_bindgen_version(), - Some("0.2.21"), - ); + assert_eq!(lock.wasm_bindgen_version(), Some("0.2.21"),); } #[test] @@ -128,8 +119,5 @@ fn it_gets_wasm_bindgen_version_from_dependencies() { ); fixture.cargo_check(); let lock = Lockfile::new(&fixture.path.join("parent")).unwrap(); - assert_eq!( - lock.wasm_bindgen_version(), - Some("0.2.21"), - ); + assert_eq!(lock.wasm_bindgen_version(), Some("0.2.21"),); }