From 97fdc94d535b0f1c3be6aad6424da12267295367 Mon Sep 17 00:00:00 2001 From: Kirk Turner Date: Fri, 19 Apr 2019 14:07:12 +0800 Subject: [PATCH 01/11] #627 - create a custom error message when wasm-bindgen fails because --web isn't available Change the return type of the child run commands so it can return a CommandError that also captures the stderr if the process fails. See if the returned output contains the 'Unknown flag: --web' error message and if so provide an alternative error that suggests upgrading --- src/bindgen.rs | 19 +++++++++- src/child.rs | 94 +++++++++++++++++++++++++++++++++++--------------- 2 files changed, 85 insertions(+), 28 deletions(-) diff --git a/src/bindgen.rs b/src/bindgen.rs index 9caf1f3b..14728e2e 100644 --- a/src/bindgen.rs +++ b/src/bindgen.rs @@ -226,10 +226,27 @@ pub fn wasm_bindgen_build( cmd.arg("--keep-debug"); } - child::run(cmd, "wasm-bindgen").context("Running the wasm-bindgen CLI")?; + let result = child::run(cmd, "wasm-bindgen"); + let result: Result<(), failure::Error> = match result { + Ok(r) => Ok(r), + Err(e) => process_error(e), + }; + result.context("Running the wasm-bindgen CLI")?; Ok(()) } +fn process_error(e: child::CommandError) -> Result<(), failure::Error> { + match &e.stderr { + Some(err) if err.trim().starts_with("Unknown flag: '--web'") => + bail!("Failed to execute `wasm-bindgen`: --web is not supported. Upgrade wasm-bindgen to a more recent version."), + Some(err) => { + eprintln!("{}", err); + bail!("{}", e.to_string()) + }, + _ => bail!("{}", e.to_string()), + } +} + /// Check if the `wasm-bindgen` dependency is locally satisfied. fn wasm_bindgen_version_check(bindgen_path: &PathBuf, dep_version: &str) -> bool { let mut cmd = Command::new(bindgen_path); diff --git a/src/child.rs b/src/child.rs index a5cfce8a..ad37db5f 100644 --- a/src/child.rs +++ b/src/child.rs @@ -3,9 +3,9 @@ //! This module helps us ensure that all child processes that we spawn get //! properly logged and their output is logged as well. -use failure::Error; use log::info; -use std::process::{Command, Stdio}; +use std::io::Error as StdError; +use std::process::{Command, ExitStatus, Stdio}; /// Return a new Command object pub fn new_command(program: &str) -> Command { @@ -23,39 +23,79 @@ pub fn new_command(program: &str) -> Command { } } -/// Run the given command and return its stdout. -pub fn run(mut command: Command, command_name: &str) -> Result<(), Error> { - info!("Running {:?}", command); +/// Error from running Command processes +/// This captures the standard error output +#[derive(Fail, Debug)] +#[fail(display = "failed to execute `{}`: {}", command_name, fail_reason)] +pub struct CommandError { + command_name: String, + fail_reason: String, + /// the output printed to stderr, if any + pub stderr: Option, +} - let status = command.status()?; +impl CommandError { + fn from_status(command_name: &str, status: ExitStatus, stderr: Option) -> CommandError { + CommandError { + command_name: command_name.to_string(), + fail_reason: format!("exited with {}", status), + stderr: stderr, + } + } - if status.success() { - Ok(()) - } else { - bail!( - "failed to execute `{}`: exited with {}", - command_name, - status - ) + fn from_error(command_name: &str, err: StdError) -> CommandError { + CommandError { + command_name: command_name.to_string(), + fail_reason: err.to_string(), + stderr: None, + } } } -/// Run the given command and return its stdout. -pub fn run_capture_stdout(mut command: Command, command_name: &str) -> Result { +/// Run the given command and return on success. +pub fn run(mut command: Command, command_name: &str) -> Result<(), CommandError> { info!("Running {:?}", command); - let output = command - .stderr(Stdio::inherit()) + let cmd_output = command .stdin(Stdio::inherit()) - .output()?; + .stdout(Stdio::inherit()) + .output(); + match cmd_output { + Ok(output) => { + if output.status.success() { + Ok(()) + } else { + Err(CommandError::from_status( + command_name, + output.status, + Some(String::from_utf8_lossy(&output.stderr).into_owned()), + )) + } + } + Err(e) => Err(CommandError::from_error(command_name, e)), + } +} - if output.status.success() { - Ok(String::from_utf8_lossy(&output.stdout).into_owned()) - } else { - bail!( - "failed to execute `{}`: exited with {}", - command_name, - output.status - ) +/// Run the given command and return its stdout. +pub fn run_capture_stdout( + mut command: Command, + command_name: &str, +) -> Result { + info!("Running {:?}", command); + + let cmd_output = command.stdin(Stdio::inherit()).output(); + match cmd_output { + Ok(output) => { + if output.status.success() { + Ok(String::from_utf8_lossy(&output.stdout).into_owned()) + } else { + Err(CommandError::from_status( + command_name, + output.status, + Some(String::from_utf8_lossy(&output.stderr).into_owned()), + )) + } + } + Err(e) => Err(CommandError::from_error(command_name, e)), } } From 64f835c0f291e442354f2ca82427171284d7e766 Mon Sep 17 00:00:00 2001 From: Kirk Turner Date: Sat, 27 Apr 2019 23:12:32 +0800 Subject: [PATCH 02/11] Get the wasm-bindgen version and display it in the error message --- src/bindgen.rs | 45 +++++++++++++++++++++++++-------------------- 1 file changed, 25 insertions(+), 20 deletions(-) diff --git a/src/bindgen.rs b/src/bindgen.rs index 14728e2e..56ea83e8 100644 --- a/src/bindgen.rs +++ b/src/bindgen.rs @@ -204,7 +204,7 @@ pub fn wasm_bindgen_build( Target::Bundler => "--browser", }; let bindgen_path = bindgen.binary("wasm-bindgen")?; - let mut cmd = Command::new(bindgen_path); + let mut cmd = Command::new(&bindgen_path); cmd.arg(&wasm_path) .arg("--out-dir") .arg(out_dir) @@ -229,42 +229,47 @@ pub fn wasm_bindgen_build( let result = child::run(cmd, "wasm-bindgen"); let result: Result<(), failure::Error> = match result { Ok(r) => Ok(r), - Err(e) => process_error(e), + Err(e) => process_error(&bindgen_path, e), }; result.context("Running the wasm-bindgen CLI")?; Ok(()) } -fn process_error(e: child::CommandError) -> Result<(), failure::Error> { +fn process_error(bindgen_path: &PathBuf, e: child::CommandError) -> Result<(), failure::Error> { match &e.stderr { - Some(err) if err.trim().starts_with("Unknown flag: '--web'") => - bail!("Failed to execute `wasm-bindgen`: --web is not supported. Upgrade wasm-bindgen to a more recent version."), + Some(err) if err.trim().starts_with("Unknown flag: '--web'") => { + let v = wasm_bindgen_get_version(bindgen_path).unwrap_or(String::from("unknown")); + bail!("Failed to execute `wasm-bindgen`: --web is not supported in version '{}'. Upgrade the wasm-bindgen dependency in Cargo.toml to version 0.2.39 or later.", v) + } Some(err) => { eprintln!("{}", err); bail!("{}", e.to_string()) - }, + } _ => bail!("{}", e.to_string()), } } /// Check if the `wasm-bindgen` dependency is locally satisfied. fn wasm_bindgen_version_check(bindgen_path: &PathBuf, dep_version: &str) -> bool { + wasm_bindgen_get_version(bindgen_path) + .map(|v| { + info!( + "Checking installed `wasm-bindgen` version == expected version: {} == {}", + v, dep_version + ); + v == dep_version + }) + .unwrap_or(false) +} + +/// Get the `wasm-bindgen` version +fn wasm_bindgen_get_version(bindgen_path: &PathBuf) -> Option { let mut cmd = Command::new(bindgen_path); cmd.arg("--version"); child::run_capture_stdout(cmd, "wasm-bindgen") - .map(|stdout| { - stdout - .trim() - .split_whitespace() - .nth(1) - .map(|v| { - info!( - "Checking installed `wasm-bindgen` version == expected version: {} == {}", - v, dep_version - ); - v == dep_version - }) - .unwrap_or(false) + .map(|stdout| match stdout.trim().split_whitespace().nth(1) { + Some(v) => return Some(v.to_owned()), + None => return None, }) - .unwrap_or(false) + .unwrap_or(None) } From fdde7291ca5a4af9f29fdd4955c98819ee8364aa Mon Sep 17 00:00:00 2001 From: Ashley Williams Date: Tue, 16 Jul 2019 14:45:13 -0500 Subject: [PATCH 03/11] test(bindgen): add test.. which passes but shouldn't --- tests/all/build.rs | 42 ++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 42 insertions(+) diff --git a/tests/all/build.rs b/tests/all/build.rs index fbd21309..74d0ef57 100644 --- a/tests/all/build.rs +++ b/tests/all/build.rs @@ -104,6 +104,48 @@ fn renamed_crate_name_works() { fixture.wasm_pack().arg("build").assert().success(); } +#[test] +fn dash_dash_web_target_has_error_on_old_bindgen() { + let fixture = utils::fixture::Fixture::new(); + fixture + .readme() + .file( + "Cargo.toml", + r#" + [package] + name = "foo" + version = "0.1.0" + authors = [] + + [lib] + crate-type = ["cdylib"] + name = 'bar' + + [dependencies] + wasm-bindgen = "0.2.37" + "#, + ) + .file( + "src/lib.rs", + r#" + extern crate wasm_bindgen; + use wasm_bindgen::prelude::*; + + #[wasm_bindgen] + pub fn one() -> u32 { 1 } + "#, + ) + .install_local_wasm_bindgen(); + fixture + .wasm_pack() + .arg("build") + .arg("--target") + .arg("web") + .assert() + .success() + .stdout(""); +} + #[test] fn it_should_build_nested_project_with_transitive_dependencies() { let fixture = utils::fixture::transitive_dependencies(); From a3541196c7ddfba0eb92e2386a4c4c6e97f84b0c Mon Sep 17 00:00:00 2001 From: Ashley Williams Date: Tue, 16 Jul 2019 14:59:20 -0500 Subject: [PATCH 04/11] feat(bidngen): we don't need to read stdout to know it will fail --- src/bindgen.rs | 21 +-------------------- src/child.rs | 30 ++++++++++++------------------ 2 files changed, 13 insertions(+), 38 deletions(-) diff --git a/src/bindgen.rs b/src/bindgen.rs index 233275da..d9611a11 100644 --- a/src/bindgen.rs +++ b/src/bindgen.rs @@ -69,29 +69,10 @@ pub fn wasm_bindgen_build( cmd.arg("--keep-debug"); } - let result = child::run(cmd, "wasm-bindgen"); - let result: Result<(), failure::Error> = match result { - Ok(r) => Ok(r), - Err(e) => process_error(&bindgen_path, e), - }; - result.context("Running the wasm-bindgen CLI")?; + child::run(cmd, "wasm-bindgen").context("Running the wasm-bindgen CLI")?; Ok(()) } -fn process_error(bindgen_path: &PathBuf, e: child::CommandError) -> Result<(), failure::Error> { - match &e.stderr { - Some(err) if err.trim().starts_with("Unknown flag: '--web'") => { - let v = wasm_bindgen_get_version(bindgen_path).unwrap_or(String::from("unknown")); - bail!("Failed to execute `wasm-bindgen`: --web is not supported in version '{}'. Upgrade the wasm-bindgen dependency in Cargo.toml to version 0.2.39 or later.", v) - } - Some(err) => { - eprintln!("{}", err); - bail!("{}", e.to_string()) - } - _ => bail!("{}", e.to_string()), - } -} - /// Check if the `wasm-bindgen` dependency is locally satisfied. fn wasm_bindgen_version_check(bindgen_path: &PathBuf, dep_version: &str) -> bool { wasm_bindgen_get_version(bindgen_path) diff --git a/src/child.rs b/src/child.rs index 2e315ac6..cd9e45e2 100644 --- a/src/child.rs +++ b/src/child.rs @@ -3,6 +3,7 @@ //! This module helps us ensure that all child processes that we spawn get //! properly logged and their output is logged as well. +use failure::Error; use install::Tool; use log::info; use std::io::Error as StdError; @@ -54,26 +55,19 @@ impl CommandError { } /// Run the given command and return on success. -pub fn run(mut command: Command, command_name: &str) -> Result<(), CommandError> { +pub fn run(mut command: Command, command_name: &str) -> Result<(), Error> { info!("Running {:?}", command); - let cmd_output = command - .stdin(Stdio::inherit()) - .stdout(Stdio::inherit()) - .output(); - match cmd_output { - Ok(output) => { - if output.status.success() { - Ok(()) - } else { - Err(CommandError::from_status( - command_name, - output.status, - Some(String::from_utf8_lossy(&output.stderr).into_owned()), - )) - } - } - Err(e) => Err(CommandError::from_error(command_name, e)), + let status = command.status()?; + + if status.success() { + Ok(()) + } else { + bail!( + "failed to execute `{}`: exited with {}", + command_name, + status + ) } } From b72048b8c74e82490bbe846cd97f28d882b9c123 Mon Sep 17 00:00:00 2001 From: Ashley Williams Date: Tue, 16 Jul 2019 15:38:41 -0500 Subject: [PATCH 05/11] feat(bindgen): check that wasm-bindgen versions match, with dummy data --- src/bindgen.rs | 31 ++++++++++-------------- src/child.rs | 64 +++++++++++--------------------------------------- 2 files changed, 27 insertions(+), 68 deletions(-) diff --git a/src/bindgen.rs b/src/bindgen.rs index d9611a11..4e2d0aab 100644 --- a/src/bindgen.rs +++ b/src/bindgen.rs @@ -69,31 +69,26 @@ pub fn wasm_bindgen_build( cmd.arg("--keep-debug"); } + let versions_match = versions_match(&bindgen_path, "0.2.37")?; + assert!(versions_match, "Something went wrong! wasm-bindgen CLI and dependency version don't match. This is likely not your fault! You should file an issue: https://github.com/rustwasm/wasm-pack/issues/new?template=bug_report.md."); + child::run(cmd, "wasm-bindgen").context("Running the wasm-bindgen CLI")?; Ok(()) } /// Check if the `wasm-bindgen` dependency is locally satisfied. -fn wasm_bindgen_version_check(bindgen_path: &PathBuf, dep_version: &str) -> bool { - wasm_bindgen_get_version(bindgen_path) - .map(|v| { - info!( - "Checking installed `wasm-bindgen` version == expected version: {} == {}", - v, dep_version - ); - v == dep_version - }) - .unwrap_or(false) +fn versions_match(cli_path: &PathBuf, dep_version: &str) -> Result { + let cli_version = cli_version(cli_path)?; + Ok(cli_version == dep_version) } -/// Get the `wasm-bindgen` version -fn wasm_bindgen_get_version(bindgen_path: &PathBuf) -> Option { +/// Get the `wasm-bindgen` CLI version +fn cli_version(bindgen_path: &PathBuf) -> Result { let mut cmd = Command::new(bindgen_path); cmd.arg("--version"); - child::run_capture_stdout(cmd, &Tool::WasmBindgen) - .map(|stdout| match stdout.trim().split_whitespace().nth(1) { - Some(v) => return Some(v.to_owned()), - None => return None, - }) - .unwrap_or(None) + let stdout = child::run_capture_stdout(cmd, &Tool::WasmBindgen)?; + match stdout.trim().split_whitespace().nth(1) { + Some(v) => Ok(v.to_string()), + None => bail!("Something went wrong! No wasm-bindgen CLI found! You should file an issue: https://github.com/rustwasm/wasm-pack/issues/new?template=bug_report.md."), + } } diff --git a/src/child.rs b/src/child.rs index cd9e45e2..3da104ab 100644 --- a/src/child.rs +++ b/src/child.rs @@ -6,8 +6,7 @@ use failure::Error; use install::Tool; use log::info; -use std::io::Error as StdError; -use std::process::{Command, ExitStatus, Stdio}; +use std::process::{Command, Stdio}; /// Return a new Command object pub fn new_command(program: &str) -> Command { @@ -25,35 +24,6 @@ pub fn new_command(program: &str) -> Command { } } -/// Error from running Command processes -/// This captures the standard error output -#[derive(Fail, Debug)] -#[fail(display = "failed to execute `{}`: {}", command_name, fail_reason)] -pub struct CommandError { - command_name: String, - fail_reason: String, - /// the output printed to stderr, if any - pub stderr: Option, -} - -impl CommandError { - fn from_status(command_name: &str, status: ExitStatus, stderr: Option) -> CommandError { - CommandError { - command_name: command_name.to_string(), - fail_reason: format!("exited with {}", status), - stderr: stderr, - } - } - - fn from_error(command_name: &str, err: StdError) -> CommandError { - CommandError { - command_name: command_name.to_string(), - fail_reason: err.to_string(), - stderr: None, - } - } -} - /// Run the given command and return on success. pub fn run(mut command: Command, command_name: &str) -> Result<(), Error> { info!("Running {:?}", command); @@ -72,27 +42,21 @@ pub fn run(mut command: Command, command_name: &str) -> Result<(), Error> { } /// Run the given command and return its stdout. -pub fn run_capture_stdout( - mut command: Command, - command_name: &Tool, -) -> Result { +pub fn run_capture_stdout(mut command: Command, command_name: &Tool) -> Result { info!("Running {:?}", command); - let cmd_display = command_name.to_string(); + let output = command + .stderr(Stdio::inherit()) + .stdin(Stdio::inherit()) + .output()?; - let cmd_output = command.stdin(Stdio::inherit()).output(); - match cmd_output { - Ok(output) => { - if output.status.success() { - Ok(String::from_utf8_lossy(&output.stdout).into_owned()) - } else { - Err(CommandError::from_status( - &cmd_display, - output.status, - Some(String::from_utf8_lossy(&output.stderr).into_owned()), - )) - } - } - Err(e) => Err(CommandError::from_error(&cmd_display, e)), + if output.status.success() { + Ok(String::from_utf8_lossy(&output.stdout).into_owned()) + } else { + bail!( + "failed to execute `{}`: exited with {}", + command_name, + output.status + ) } } From c65e1ecc3d00fa6d9cf41119f6d3352caab636be Mon Sep 17 00:00:00 2001 From: Ashley Williams Date: Tue, 16 Jul 2019 15:59:54 -0500 Subject: [PATCH 06/11] test(bindgen): remember how versions in cargo.toml work --- tests/all/build.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/all/build.rs b/tests/all/build.rs index 74d0ef57..8296d6a3 100644 --- a/tests/all/build.rs +++ b/tests/all/build.rs @@ -122,7 +122,7 @@ fn dash_dash_web_target_has_error_on_old_bindgen() { name = 'bar' [dependencies] - wasm-bindgen = "0.2.37" + wasm-bindgen = "=0.2.37" "#, ) .file( From af03d1d3e2364775b5116f6932af8565f3c78c01 Mon Sep 17 00:00:00 2001 From: Ashley Williams Date: Tue, 16 Jul 2019 16:01:50 -0500 Subject: [PATCH 07/11] fix(bindgen): remove unused import --- src/bindgen.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/src/bindgen.rs b/src/bindgen.rs index 4e2d0aab..7dfb8ca5 100644 --- a/src/bindgen.rs +++ b/src/bindgen.rs @@ -5,7 +5,6 @@ use child; use command::build::{BuildProfile, Target}; use failure::{self, ResultExt}; use install::Tool; -use log::info; use manifest::CrateData; use std::path::{Path, PathBuf}; use std::process::Command; From 4e255675f0c1328ffe3206f004f576815654daf1 Mon Sep 17 00:00:00 2001 From: Ashley Williams Date: Tue, 16 Jul 2019 16:02:45 -0500 Subject: [PATCH 08/11] fix(clippy): remove unnecessary return --- src/manifest/mod.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/manifest/mod.rs b/src/manifest/mod.rs index 80dc0682..cdf86b18 100644 --- a/src/manifest/mod.rs +++ b/src/manifest/mod.rs @@ -368,12 +368,12 @@ impl CrateData { .position(|pkg| pkg.name == manifest.package.name) .ok_or_else(|| format_err!("failed to find package in metadata"))?; - return Ok(CrateData { + Ok(CrateData { data, manifest, current_idx, out_name, - }); + }) } /// Read the `manifest_path` file and deserializes it using the toml Deserializer. From c293720fe192ff3b3e7e0b3b00970b40c1d912bd Mon Sep 17 00:00:00 2001 From: Ashley Williams Date: Tue, 16 Jul 2019 16:28:28 -0500 Subject: [PATCH 09/11] feat(bindgen): use version check function from install --- src/bindgen.rs | 19 ++++--------------- src/install/mod.rs | 2 +- 2 files changed, 5 insertions(+), 16 deletions(-) diff --git a/src/bindgen.rs b/src/bindgen.rs index 7dfb8ca5..fe506882 100644 --- a/src/bindgen.rs +++ b/src/bindgen.rs @@ -2,6 +2,7 @@ use binary_install::Download; use child; +use crate::install; use command::build::{BuildProfile, Target}; use failure::{self, ResultExt}; use install::Tool; @@ -68,7 +69,7 @@ pub fn wasm_bindgen_build( cmd.arg("--keep-debug"); } - let versions_match = versions_match(&bindgen_path, "0.2.37")?; + let versions_match = install::check_version(&Tool::WasmBindgen, &bindgen_path, "0.2.37")?; assert!(versions_match, "Something went wrong! wasm-bindgen CLI and dependency version don't match. This is likely not your fault! You should file an issue: https://github.com/rustwasm/wasm-pack/issues/new?template=bug_report.md."); child::run(cmd, "wasm-bindgen").context("Running the wasm-bindgen CLI")?; @@ -76,18 +77,6 @@ pub fn wasm_bindgen_build( } /// Check if the `wasm-bindgen` dependency is locally satisfied. -fn versions_match(cli_path: &PathBuf, dep_version: &str) -> Result { - let cli_version = cli_version(cli_path)?; - Ok(cli_version == dep_version) -} - -/// Get the `wasm-bindgen` CLI version -fn cli_version(bindgen_path: &PathBuf) -> Result { - let mut cmd = Command::new(bindgen_path); - cmd.arg("--version"); - let stdout = child::run_capture_stdout(cmd, &Tool::WasmBindgen)?; - match stdout.trim().split_whitespace().nth(1) { - Some(v) => Ok(v.to_string()), - None => bail!("Something went wrong! No wasm-bindgen CLI found! You should file an issue: https://github.com/rustwasm/wasm-pack/issues/new?template=bug_report.md."), - } +fn supports_web_target(cli_path: &PathBuf, dep_version: &str) -> Result { + unimplemented!(); } diff --git a/src/install/mod.rs b/src/install/mod.rs index 494bc850..d4c71222 100644 --- a/src/install/mod.rs +++ b/src/install/mod.rs @@ -63,7 +63,7 @@ pub fn download_prebuilt_or_cargo_install( } /// Check if the tool dependency is locally satisfied. -fn check_version( +pub fn check_version( tool: &Tool, path: &PathBuf, expected_version: &str, From 22a63ea86fafd14e3a338acbc506f9693c0f3e37 Mon Sep 17 00:00:00 2001 From: Ashley Williams Date: Tue, 16 Jul 2019 16:44:31 -0500 Subject: [PATCH 10/11] feat(install): break out get cli version function --- src/install/mod.rs | 32 ++++++++++++++++---------------- 1 file changed, 16 insertions(+), 16 deletions(-) diff --git a/src/install/mod.rs b/src/install/mod.rs index d4c71222..7009c283 100644 --- a/src/install/mod.rs +++ b/src/install/mod.rs @@ -75,24 +75,24 @@ pub fn check_version( expected_version.to_string() }; + let v = get_cli_version(tool, path)?; + info!( + "Checking installed `{}` version == expected version: {} == {}", + tool, v, &expected_version + ); + Ok(v == expected_version) +} + +/// Fetches the version of a CLI tool +pub fn get_cli_version(tool: &Tool, path: &PathBuf) -> Result { let mut cmd = Command::new(path); cmd.arg("--version"); - child::run_capture_stdout(cmd, tool) - .map(|stdout| { - stdout - .trim() - .split_whitespace() - .nth(1) - .map(|v| { - info!( - "Checking installed `{}` version == expected version: {} == {}", - tool, v, &expected_version - ); - Ok(v == expected_version) - }) - .unwrap_or(Ok(false)) - }) - .unwrap_or(Ok(false)) + let stdout = child::run_capture_stdout(cmd, tool)?; + let version = stdout.trim().split_whitespace().nth(1); + match version { + Some(v) => Ok(v.to_string()), + None => bail!("Something went wrong! We couldn't determine your version of the wasm-bindgen CLI. We were supposed to set that up for you, so it's likely not your fault! You should file an issue: https://github.com/rustwasm/wasm-pack/issues/new?template=bug_report.md.") + } } /// Downloads a precompiled copy of the tool, if available. From 032c9f4cbd0e8361a57135daa64c145e76c0e63a Mon Sep 17 00:00:00 2001 From: Ashley Williams Date: Tue, 16 Jul 2019 17:29:08 -0500 Subject: [PATCH 11/11] feat(bindgen): check wasm-bindgen cli version when running target web --- Cargo.lock | 1 + Cargo.toml | 1 + src/bindgen.rs | 27 ++++++++++++++++++--------- src/lib.rs | 1 + tests/all/build.rs | 12 +++++++++--- 5 files changed, 30 insertions(+), 12 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 4e26d13d..8a272408 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1987,6 +1987,7 @@ dependencies = [ "parking_lot 0.6.4 (registry+https://github.com/rust-lang/crates.io-index)", "predicates 1.0.0 (registry+https://github.com/rust-lang/crates.io-index)", "reqwest 0.9.14 (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.89 (registry+https://github.com/rust-lang/crates.io-index)", "serde_derive 1.0.89 (registry+https://github.com/rust-lang/crates.io-index)", "serde_ignored 0.0.4 (registry+https://github.com/rust-lang/crates.io-index)", diff --git a/Cargo.toml b/Cargo.toml index e12044a2..d85d70f6 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -24,6 +24,7 @@ log = "0.4.6" openssl = { version = '0.10.11', optional = true } parking_lot = "0.6" reqwest = "0.9.14" +semver = "0.9.0" serde = "1.0.74" serde_derive = "1.0.74" serde_ignored = "0.0.4" diff --git a/src/bindgen.rs b/src/bindgen.rs index fe506882..898743dc 100644 --- a/src/bindgen.rs +++ b/src/bindgen.rs @@ -2,11 +2,11 @@ use binary_install::Download; use child; -use crate::install; use command::build::{BuildProfile, Target}; use failure::{self, ResultExt}; -use install::Tool; +use install; use manifest::CrateData; +use semver; use std::path::{Path, PathBuf}; use std::process::Command; @@ -40,13 +40,20 @@ pub fn wasm_bindgen_build( } else { "--typescript" }; + let bindgen_path = bindgen.binary("wasm-bindgen")?; let target_arg = match target { Target::Nodejs => "--nodejs", Target::NoModules => "--no-modules", - Target::Web => "--web", + Target::Web => { + if supports_web_target(&bindgen_path)? { + "--web" + } else { + bail!("Your current version of wasm-bindgen does not support the 'web' target. Please update your project to wasm-bindgen version >= 0.2.39.") + } + } Target::Bundler => "--browser", }; - let bindgen_path = bindgen.binary("wasm-bindgen")?; + let mut cmd = Command::new(&bindgen_path); cmd.arg(&wasm_path) .arg("--out-dir") @@ -69,14 +76,16 @@ pub fn wasm_bindgen_build( cmd.arg("--keep-debug"); } - let versions_match = install::check_version(&Tool::WasmBindgen, &bindgen_path, "0.2.37")?; - assert!(versions_match, "Something went wrong! wasm-bindgen CLI and dependency version don't match. This is likely not your fault! You should file an issue: https://github.com/rustwasm/wasm-pack/issues/new?template=bug_report.md."); - child::run(cmd, "wasm-bindgen").context("Running the wasm-bindgen CLI")?; Ok(()) } /// Check if the `wasm-bindgen` dependency is locally satisfied. -fn supports_web_target(cli_path: &PathBuf, dep_version: &str) -> Result { - unimplemented!(); +fn supports_web_target(cli_path: &PathBuf) -> Result { + let cli_version = semver::Version::parse(&install::get_cli_version( + &install::Tool::WasmBindgen, + cli_path, + )?)?; + let expected_version = semver::Version::parse("0.2.39")?; + Ok(cli_version >= expected_version) } diff --git a/src/lib.rs b/src/lib.rs index c327d300..7eff5a32 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -9,6 +9,7 @@ extern crate strsim; extern crate failure; extern crate glob; extern crate parking_lot; +extern crate semver; extern crate serde; extern crate which; #[macro_use] diff --git a/tests/all/build.rs b/tests/all/build.rs index 8296d6a3..4ae4f9ce 100644 --- a/tests/all/build.rs +++ b/tests/all/build.rs @@ -136,14 +136,20 @@ fn dash_dash_web_target_has_error_on_old_bindgen() { "#, ) .install_local_wasm_bindgen(); - fixture + let cmd = fixture .wasm_pack() .arg("build") .arg("--target") .arg("web") .assert() - .success() - .stdout(""); + .failure(); + let output = String::from_utf8(cmd.get_output().stderr.clone()).unwrap(); + + assert!( + output.contains("0.2.39"), + "Output did not contain '0.2.39', output was {}", + output + ); } #[test]