Skip to content

Commit

Permalink
feat(lockfiles): Use Cargo.lock to identify wasm-bindgen versions
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
data-pup authored and fitzgen committed Sep 20, 2018
1 parent fa61f90 commit 790ff99
Show file tree
Hide file tree
Showing 11 changed files with 295 additions and 141 deletions.
24 changes: 24 additions & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down
7 changes: 4 additions & 3 deletions src/command/build.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -155,18 +156,18 @@ impl Build {
}
match &mode {
BuildMode::Normal => steps![
step_check_crate_config,
step_add_wasm_target,
step_build_wasm,
step_check_crate_config,
step_create_dir,
step_create_json,
step_copy_readme,
step_install_wasm_bindgen,
step_run_wasm_bindgen,
],
BuildMode::Noinstall => steps![
step_check_crate_config,
step_build_wasm,
step_check_crate_config,
step_create_dir,
step_create_json,
step_copy_readme,
Expand Down Expand Up @@ -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,
Expand Down
7 changes: 4 additions & 3 deletions src/command/test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -183,9 +184,9 @@ impl Test {
}
match self.mode {
BuildMode::Normal => steps![
step_check_crate_config,
step_add_wasm_target,
step_build_tests,
step_check_crate_config,
step_install_wasm_bindgen,
step_test_node if self.node,
step_get_chromedriver if self.chrome && self.chromedriver.is_none(),
Expand All @@ -196,8 +197,8 @@ impl Test {
step_test_safari if self.safari,
],
BuildMode::Noinstall => steps![
step_check_crate_config,
step_build_tests,
step_check_crate_config,
step_install_wasm_bindgen,
step_test_node if self.node,
step_get_chromedriver if self.chrome && self.chromedriver.is_none(),
Expand Down Expand Up @@ -238,7 +239,7 @@ 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)?;
let bindgen_version = lockfile::get_wasm_bindgen_version(&self.crate_path)?;
info!(&log, "Installing wasm-bindgen-cli...");

let install_permitted = match self.mode {
Expand Down
2 changes: 2 additions & 0 deletions src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
#![deny(missing_docs)]

extern crate cargo_metadata;
extern crate console;
extern crate curl;
#[macro_use]
Expand Down Expand Up @@ -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;
Expand Down
92 changes: 92 additions & 0 deletions src/lockfile.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,92 @@
//! 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<Package>,
}

/// This struct represents a single package entry in `Cargo.lock`
#[derive(Clone, Debug, Deserialize)]
struct Package {
name: String,
version: String,
source: Option<String>,
}

impl Lockfile {
fn get_package_version(&self, package: &str) -> Option<String> {
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<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 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(|| {
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<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)
}

/// 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, 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;
// 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)
}
}
98 changes: 5 additions & 93 deletions src/manifest/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,9 +8,9 @@ use std::io::prelude::*;
use std::path::Path;

use self::npm::{repository::Repository, CommonJSPackage, ESModulesPackage, NpmPackage};
use console::style;
use emoji;
use error::Error;
use lockfile;
use progressbar::Step;
use serde_json;
use toml;
Expand All @@ -25,31 +25,6 @@ struct CargoManifest {
lib: Option<CargoLib>,
}

fn normalize_dependency_name(dep: &str) -> String {
dep.replace("-", "_")
}

fn normalize_dependencies(
deps: HashMap<String, CargoDependency>,
) -> HashMap<String, CargoDependency> {
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,
Expand Down Expand Up @@ -111,9 +86,7 @@ fn read_cargo_toml(path: &Path) -> Result<CargoManifest, Error> {
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)
}

Expand Down Expand Up @@ -237,16 +210,16 @@ pub fn check_crate_config(path: &Path, step: &Step) -> Result<(), Error> {
}

fn check_wasm_bindgen(path: &Path) -> Result<(), Error> {
get_wasm_bindgen_version(path)?;
lockfile::get_wasm_bindgen_version(path)?;
Ok(())
}

fn check_wasm_bindgen_test(path: &Path) -> Result<(), Error> {
let expected_version = get_wasm_bindgen_version(path)?;
let expected_version = lockfile::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 let Ok(actual_version) = lockfile::get_wasm_bindgen_test_version(path) {
if expected_version != actual_version {
return Error::crate_config(&format!(
"The `wasm-bindgen-test` dependency version ({}) must match \
Expand All @@ -273,64 +246,3 @@ fn check_crate_type(path: &Path) -> Result<(), Error> {
crate-type = [\"cdylib\"]"
)
}

fn get_dependency_version(
dependencies: Option<&HashMap<String, CargoDependency>>,
dependency: &str,
dependencies_section_name: &str,
version_suggestion: &str,
) -> Result<String, Error> {
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<String, Error> {
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<String, Error> {
let toml = read_cargo_toml(path)?;
get_dependency_version(
toml.dev_dependencies.as_ref(),
"wasm-bindgen-test",
"dev-dependencies",
"0.2",
)
}
Loading

0 comments on commit 790ff99

Please sign in to comment.