More effort towards "1.0 output" goals

This commit moves wasm-pack further along the spectrum towards the 1.0
output previously discussed at the last work week. The changes here are:

* Steps which execute near instantaneously no longer print informational
  messages by default.
* Long-running steps like downloading chromedriver/geckodriver now only
  print if they actually perform a download.
* The "add wasm target" step now prints nothing if the wasm target is
  already installed.
* Child process output is no longer captured and is redirected natively
  to the terminal, granting colors from rustc/Cargo as well as Cargo's
  progress bar during a build.
master
Alex Crichton 6 years ago
parent 64239747a0
commit d3d2a4cc28
  1. 1547
      Cargo.lock
  2. 4
      src/bindgen.rs
  3. 21
      src/build.rs
  4. 167
      src/child.rs
  5. 31
      src/command/build.rs
  6. 33
      src/command/test.rs
  7. 7
      src/command/utils.rs
  8. 9
      src/license.rs
  9. 9
      src/manifest/mod.rs
  10. 2
      src/progressbar.rs
  11. 12
      src/readme.rs
  12. 42
      src/test/webdriver.rs

1547
Cargo.lock generated

File diff suppressed because it is too large Load Diff

@ -178,11 +178,7 @@ pub fn wasm_bindgen_build(
disable_dts: bool,
target: &Target,
profile: BuildProfile,
step: &Step,
) -> Result<(), failure::Error> {
let msg = format!("{}Running wasm-bindgen...", emoji::RUNNER);
PBAR.step(step, &msg);
let release_or_debug = match profile {
BuildProfile::Release | BuildProfile::Profiling => "release",
BuildProfile::Dev => "debug",

@ -11,9 +11,7 @@ use std::str;
use PBAR;
/// Ensure that `rustc` is present and that it is >= 1.30.0
pub fn check_rustc_version(step: &Step) -> Result<String, Error> {
let msg = format!("{}Checking `rustc` version...", emoji::CRAB);
PBAR.step(step, &msg);
pub fn check_rustc_version() -> Result<String, Error> {
let local_minor_version = rustc_minor_version();
match local_minor_version {
Some(mv) => {
@ -52,7 +50,19 @@ fn rustc_minor_version() -> Option<u32> {
/// Ensure that `rustup` has the `wasm32-unknown-unknown` target installed for
/// current toolchain
pub fn rustup_add_wasm_target(step: &Step) -> Result<(), Error> {
let msg = format!("{}Adding the Wasm target...", emoji::TARGET);
let mut cmd = Command::new("rustc");
cmd.arg("--print").arg("sysroot");
let output = child::run(cmd, "rustc").context("Learning about rustc's sysroot")?;
let sysroot = Path::new(output.trim());
// If this exists then we for sure have a wasm32 target so there's no need
// to progress further.
if sysroot.join("lib/rustlib/wasm32-unknown-unknown").exists() {
return Ok(());
}
// ... otherwise fall back to rustup to add the target
let msg = format!("{}Adding Wasm target...", emoji::TARGET);
PBAR.step(step, &msg);
let mut cmd = Command::new("rustup");
cmd.arg("target").arg("add").arg("wasm32-unknown-unknown");
@ -64,11 +74,8 @@ pub fn rustup_add_wasm_target(step: &Step) -> Result<(), Error> {
pub fn cargo_build_wasm(
path: &Path,
profile: BuildProfile,
step: &Step,
extra_options: &Vec<String>,
) -> Result<(), Error> {
let msg = format!("{}Compiling to Wasm...", emoji::CYCLONE);
PBAR.step(step, &msg);
let mut cmd = Command::new("cargo");
cmd.current_dir(path).arg("build").arg("--lib");
match profile {

@ -5,21 +5,7 @@
use failure::Error;
use log::info;
use std::{
io::{self, Read},
mem,
process::{Command, Stdio},
string,
sync::mpsc,
thread,
};
use PBAR;
#[derive(Debug)]
enum OutputFragment {
Stdout(Vec<u8>),
Stderr(Vec<u8>),
}
use std::process::{Command, Stdio};
/// Return a new Command object
pub fn new_command(program: &str) -> Command {
@ -37,151 +23,22 @@ pub fn new_command(program: &str) -> Command {
}
}
/// Read data from the give reader and send it as an `OutputFragment` over the
/// given sender.
fn read_and_send<R, F>(
mut reader: R,
sender: &mpsc::Sender<OutputFragment>,
mut map: F,
) -> io::Result<()>
where
R: Read,
F: FnMut(Vec<u8>) -> OutputFragment,
{
let mut buf = vec![0; 1024];
loop {
match reader.read(&mut buf) {
Err(e) => {
if e.kind() == io::ErrorKind::Interrupted {
continue;
} else {
return Err(e);
}
}
Ok(0) => return Ok(()),
Ok(n) => {
buf.truncate(n);
let buf = mem::replace(&mut buf, vec![0; 1024]);
sender.send(map(buf)).unwrap();
}
}
}
}
/// Accumulates output from a stream of output fragments and calls a callback on
/// each complete line as it is accumulating.
struct OutputAccumulator<F> {
result: String,
in_progress: Vec<u8>,
on_each_line: F,
}
impl<F> OutputAccumulator<F>
where
F: FnMut(&str),
{
/// Construct a new output accumulator with the given `on_each_line`
/// callback.
fn new(on_each_line: F) -> OutputAccumulator<F> {
OutputAccumulator {
result: String::new(),
in_progress: Vec::new(),
on_each_line,
}
}
/// Add another fragment of output to the accumulation, calling the
/// `on_each_line` callback for any complete lines we accumulate.
fn push(&mut self, fragment: Vec<u8>) -> Result<(), string::FromUtf8Error> {
debug_assert!(!fragment.is_empty());
self.in_progress.extend(fragment);
if let Some((last_newline, _)) = self
.in_progress
.iter()
.cloned()
.enumerate()
.rev()
.find(|(_, ch)| *ch == b'\n')
{
let next_in_progress: Vec<u8> = self.in_progress[last_newline + 1..].to_vec();
let mut these_lines = mem::replace(&mut self.in_progress, next_in_progress);
these_lines.truncate(last_newline + 1);
let these_lines = String::from_utf8(these_lines)?;
for line in these_lines.lines() {
(self.on_each_line)(line);
}
self.result.push_str(&these_lines);
}
Ok(())
}
/// Finish accumulation, run the `on_each_line` callback on the final line
/// (if any), and return the accumulated output.
fn finish(mut self) -> Result<String, string::FromUtf8Error> {
if !self.in_progress.is_empty() {
let last_line = String::from_utf8(self.in_progress)?;
(self.on_each_line)(&last_line);
self.result.push_str(&last_line);
}
Ok(self.result)
}
}
/// Run the given command and return its stdout.
pub fn run(mut command: Command, command_name: &str) -> Result<String, Error> {
info!("Running {:?}", command);
let mut child = command
.stdout(Stdio::piped())
.stderr(Stdio::piped())
.spawn()?;
let stdout = child.stdout.take().unwrap();
let stderr = child.stderr.take().unwrap();
let (send, recv) = mpsc::channel();
let stdout_send = send.clone();
let stderr_send = send;
// Because pipes have a fixed-size buffer, we need to keep reading stdout
// and stderr on a separate thread to avoid potential dead locks with
// waiting on the child process.
let stdout_handle =
thread::spawn(move || read_and_send(stdout, &stdout_send, OutputFragment::Stdout));
let stderr_handle =
thread::spawn(move || read_and_send(stderr, &stderr_send, OutputFragment::Stderr));
let mut stdout = OutputAccumulator::new(|line| {
info!("{} (stdout): {}", command_name, line);
PBAR.message(line)
});
let mut stderr = OutputAccumulator::new(|line| {
info!("{} (stderr): {}", command_name, line);
PBAR.message(line)
});
for output in recv {
match output {
OutputFragment::Stdout(line) => stdout.push(line)?,
OutputFragment::Stderr(line) => stderr.push(line)?,
};
}
let stdout = stdout.finish()?;
let stderr = stderr.finish()?;
// Join the threads reading the child's output to make sure the finish OK.
stdout_handle.join().unwrap()?;
stderr_handle.join().unwrap()?;
let output = command
.stderr(Stdio::inherit())
.stdin(Stdio::inherit())
.output()?;
let exit = child.wait()?;
if exit.success() {
Ok(stdout)
if output.status.success() {
Ok(String::from_utf8_lossy(&output.stdout).into_owned())
} else {
drop((stdout, stderr));
bail!("failed to execute `{}`: exited with {}", command_name, exit)
bail!(
"failed to execute `{}`: exited with {}",
command_name,
output.status
)
}
}

@ -297,17 +297,17 @@ impl Build {
}
}
fn step_check_rustc_version(&mut self, step: &Step) -> Result<(), Error> {
fn step_check_rustc_version(&mut self, _step: &Step) -> Result<(), Error> {
info!("Checking rustc version...");
let version = build::check_rustc_version(step)?;
let version = build::check_rustc_version()?;
let msg = format!("rustc version is {}.", version);
info!("{}", &msg);
Ok(())
}
fn step_check_crate_config(&mut self, step: &Step) -> Result<(), Error> {
fn step_check_crate_config(&mut self, _step: &Step) -> Result<(), Error> {
info!("Checking crate configuration...");
self.crate_data.check_crate_config(step)?;
self.crate_data.check_crate_config()?;
info!("Crate is correctly configured.");
Ok(())
}
@ -319,9 +319,9 @@ impl Build {
Ok(())
}
fn step_build_wasm(&mut self, step: &Step) -> Result<(), Error> {
fn step_build_wasm(&mut self, _step: &Step) -> Result<(), Error> {
info!("Building wasm...");
build::cargo_build_wasm(&self.crate_path, self.profile, step, &self.extra_options)?;
build::cargo_build_wasm(&self.crate_path, self.profile, &self.extra_options)?;
info!(
"wasm built at {:#?}.",
@ -334,21 +334,19 @@ impl Build {
Ok(())
}
fn step_create_dir(&mut self, step: &Step) -> Result<(), Error> {
fn step_create_dir(&mut self, _step: &Step) -> Result<(), Error> {
info!("Creating a pkg directory...");
create_pkg_dir(&self.out_dir, step)?;
create_pkg_dir(&self.out_dir)?;
info!("Created a pkg directory at {:#?}.", &self.crate_path);
Ok(())
}
fn step_create_json(&mut self, step: &Step) -> Result<(), Error> {
info!("Writing a package.json...");
fn step_create_json(&mut self, _step: &Step) -> Result<(), Error> {
self.crate_data.write_package_json(
&self.out_dir,
&self.scope,
self.disable_dts,
&self.target,
step,
)?;
info!(
"Wrote a package.json at {:#?}.",
@ -357,16 +355,16 @@ impl Build {
Ok(())
}
fn step_copy_readme(&mut self, step: &Step) -> Result<(), Error> {
fn step_copy_readme(&mut self, _step: &Step) -> Result<(), Error> {
info!("Copying readme from crate...");
readme::copy_from_crate(&self.crate_path, &self.out_dir, step)?;
readme::copy_from_crate(&self.crate_path, &self.out_dir)?;
info!("Copied readme from crate to {:#?}.", &self.out_dir);
Ok(())
}
fn step_copy_license(&mut self, step: &Step) -> Result<(), failure::Error> {
fn step_copy_license(&mut self, _step: &Step) -> Result<(), failure::Error> {
info!("Copying license from crate...");
license::copy_from_crate(&self.crate_data, &self.crate_path, &self.out_dir, step)?;
license::copy_from_crate(&self.crate_data, &self.crate_path, &self.out_dir)?;
info!("Copied license from crate to {:#?}.", &self.out_dir);
Ok(())
}
@ -388,7 +386,7 @@ impl Build {
Ok(())
}
fn step_run_wasm_bindgen(&mut self, step: &Step) -> Result<(), Error> {
fn step_run_wasm_bindgen(&mut self, _step: &Step) -> Result<(), Error> {
info!("Building the wasm bindings...");
bindgen::wasm_bindgen_build(
&self.crate_data,
@ -397,7 +395,6 @@ impl Build {
self.disable_dts,
&self.target,
self.profile,
step,
)?;
info!("wasm bindings were built at {:#?}.", &self.out_dir);
Ok(())

@ -7,7 +7,6 @@ use build;
use cache;
use command::utils::set_crate_path;
use console::style;
use emoji;
use failure::Error;
use lockfile::Lockfile;
use log::info;
@ -16,7 +15,6 @@ use progressbar::Step;
use std::path::PathBuf;
use std::time::Instant;
use test::{self, webdriver};
use PBAR;
#[derive(Debug, Default, StructOpt)]
/// Everything required to configure the `wasm-pack test` command.
@ -232,9 +230,9 @@ impl Test {
}
}
fn step_check_rustc_version(&mut self, step: &Step) -> Result<(), Error> {
fn step_check_rustc_version(&mut self, _step: &Step) -> Result<(), Error> {
info!("Checking rustc version...");
let _ = build::check_rustc_version(step)?;
let _ = build::check_rustc_version()?;
info!("Rustc version is correct.");
Ok(())
}
@ -246,12 +244,9 @@ impl Test {
Ok(())
}
fn step_build_tests(&mut self, step: &Step) -> Result<(), Error> {
fn step_build_tests(&mut self, _step: &Step) -> Result<(), Error> {
info!("Compiling tests to wasm...");
let msg = format!("{}Compiling tests to Wasm...", emoji::CYCLONE);
PBAR.step(step, &msg);
build::cargo_build_wasm_tests(&self.crate_path, !self.release)?;
info!("Finished compiling tests to wasm.");
@ -301,10 +296,9 @@ impl Test {
Ok(())
}
fn step_test_node(&mut self, step: &Step) -> Result<(), Error> {
fn step_test_node(&mut self, _step: &Step) -> Result<(), Error> {
assert!(self.node);
info!("Running tests in node...");
PBAR.step(step, "Running tests in node...");
test::cargo_test_wasm(
&self.crate_path,
self.release,
@ -319,19 +313,17 @@ impl Test {
}
fn step_get_chromedriver(&mut self, step: &Step) -> Result<(), Error> {
PBAR.step(step, "Getting chromedriver...");
assert!(self.chrome && self.chromedriver.is_none());
self.chromedriver = Some(webdriver::get_or_install_chromedriver(
&self.cache,
self.mode,
step,
)?);
Ok(())
}
fn step_test_chrome(&mut self, step: &Step) -> Result<(), Error> {
PBAR.step(step, "Running tests in Chrome...");
fn step_test_chrome(&mut self, _step: &Step) -> Result<(), Error> {
let chromedriver = self.chromedriver.as_ref().unwrap().display().to_string();
let chromedriver = chromedriver.as_str();
info!(
@ -361,19 +353,17 @@ impl Test {
}
fn step_get_geckodriver(&mut self, step: &Step) -> Result<(), Error> {
PBAR.step(step, "Getting geckodriver...");
assert!(self.firefox && self.geckodriver.is_none());
self.geckodriver = Some(webdriver::get_or_install_geckodriver(
&self.cache,
self.mode,
step,
)?);
Ok(())
}
fn step_test_firefox(&mut self, step: &Step) -> Result<(), Error> {
PBAR.step(step, "Running tests in Firefox...");
fn step_test_firefox(&mut self, _step: &Step) -> Result<(), Error> {
let geckodriver = self.geckodriver.as_ref().unwrap().display().to_string();
let geckodriver = geckodriver.as_str();
info!(
@ -402,17 +392,14 @@ impl Test {
Ok(())
}
fn step_get_safaridriver(&mut self, step: &Step) -> Result<(), Error> {
PBAR.step(step, "Getting safaridriver...");
fn step_get_safaridriver(&mut self, _step: &Step) -> Result<(), Error> {
assert!(self.safari && self.safaridriver.is_none());
self.safaridriver = Some(webdriver::get_safaridriver()?);
Ok(())
}
fn step_test_safari(&mut self, step: &Step) -> Result<(), Error> {
PBAR.step(step, "Running tests in Safari...");
fn step_test_safari(&mut self, _step: &Step) -> Result<(), Error> {
let safaridriver = self.safaridriver.as_ref().unwrap().display().to_string();
let safaridriver = safaridriver.as_str();
info!(

@ -1,13 +1,10 @@
//! Utility functions for commands.
use emoji;
use failure;
use progressbar::Step;
use std::fs;
use std::path::{Path, PathBuf};
use std::time::Duration;
use walkdir::WalkDir;
use PBAR;
/// If an explicit path is given, then use it, otherwise assume the current
/// directory is the crate path.
@ -16,9 +13,7 @@ pub fn set_crate_path(path: Option<PathBuf>) -> Result<PathBuf, failure::Error>
}
/// Construct our `pkg` directory in the crate.
pub fn create_pkg_dir(out_dir: &Path, step: &Step) -> Result<(), failure::Error> {
let msg = format!("{}Creating a pkg directory...", emoji::FOLDER);
PBAR.step(step, &msg);
pub fn create_pkg_dir(out_dir: &Path) -> Result<(), failure::Error> {
fs::create_dir_all(&out_dir)?;
fs::write(out_dir.join(".gitignore"), "*")?;
Ok(())

@ -4,10 +4,8 @@ use failure;
use std::fs;
use std::path::Path;
use emoji;
use glob::glob;
use manifest::CrateData;
use progressbar::Step;
use PBAR;
fn glob_license_files(path: &Path) -> Result<Vec<String>, failure::Error> {
@ -45,7 +43,6 @@ pub fn copy_from_crate(
crate_data: &CrateData,
path: &Path,
out_dir: &Path,
step: &Step,
) -> Result<(), failure::Error> {
assert!(
fs::metadata(path).ok().map_or(false, |m| m.is_dir()),
@ -59,8 +56,6 @@ pub fn copy_from_crate(
match (crate_data.crate_license(), crate_data.crate_license_file()) {
(Some(_), _) => {
let msg = format!("{}Copying over your LICENSE...", emoji::DANCERS);
PBAR.step(step, &msg);
let license_files = glob_license_files(path);
match license_files {
@ -87,9 +82,7 @@ pub fn copy_from_crate(
PBAR.info("origin crate has no LICENSE");
}
}
(None, None) => {
PBAR.step(step, "No LICENSE found in Cargo.toml, skipping...");
}
(None, None) => {}
};
Ok(())

@ -14,7 +14,6 @@ use cargo_metadata::Metadata;
use command::build::{BuildProfile, Target};
use emoji;
use failure::{Error, ResultExt};
use progressbar::Step;
use serde::{self, Deserialize};
use serde_json;
use std::collections::BTreeSet;
@ -312,9 +311,7 @@ impl CrateData {
}
/// Check that the crate the given path is properly configured.
pub fn check_crate_config(&self, step: &Step) -> Result<(), Error> {
let msg = format!("{}Checking crate configuration...", emoji::WRENCH);
PBAR.step(&step, &msg);
pub fn check_crate_config(&self) -> Result<(), Error> {
self.check_crate_type()?;
Ok(())
}
@ -378,11 +375,7 @@ impl CrateData {
scope: &Option<String>,
disable_dts: bool,
target: &Target,
step: &Step,
) -> Result<(), Error> {
let msg = format!("{}Writing a package.json...", emoji::MEMO);
PBAR.step(step, &msg);
let pkg_file_path = out_dir.join("package.json");
let npm_data = match target {
Target::Nodejs => self.to_commonjs(scope, disable_dts, out_dir),

@ -81,7 +81,7 @@ impl Step {
impl fmt::Display for Step {
fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
write!(f, "[{}/{}]", self.current, self.total)
write!(f, "{}", emoji::INFO)
}
}

@ -1,15 +1,13 @@
//! Generating `README` files for the packaged wasm.
use failure;
use failure::{self, ResultExt};
use std::fs;
use std::path::Path;
use emoji;
use progressbar::Step;
use PBAR;
/// Copy the crate's README into the `pkg` directory.
pub fn copy_from_crate(path: &Path, out_dir: &Path, step: &Step) -> Result<(), failure::Error> {
pub fn copy_from_crate(path: &Path, out_dir: &Path) -> Result<(), failure::Error> {
assert!(
fs::metadata(path).ok().map_or(false, |m| m.is_dir()),
"crate directory should exist"
@ -19,11 +17,11 @@ pub fn copy_from_crate(path: &Path, out_dir: &Path, step: &Step) -> Result<(), f
"crate's pkg directory should exist"
);
let msg = format!("{}Copying over your README...", emoji::DANCERS);
PBAR.step(step, &msg);
let crate_readme_path = path.join("README.md");
let new_readme_path = out_dir.join("README.md");
if fs::copy(&crate_readme_path, &new_readme_path).is_err() {
if crate_readme_path.exists() {
fs::copy(&crate_readme_path, &new_readme_path).context("failed to copy README")?;
} else {
PBAR.warn("origin crate has no README");
}
Ok(())

@ -1,16 +1,38 @@
//! Getting WebDriver client binaries.
use crate::progressbar::Step;
use binary_install::Cache;
use command::build::BuildMode;
use failure;
use std::path::PathBuf;
use target;
use PBAR;
fn get_and_notify(
cache: &Cache,
installation_allowed: bool,
name: &str,
url: &str,
step: &Step,
) -> Result<Option<PathBuf>, failure::Error> {
if let Some(dl) = cache.download(false, name, &[name], &url)? {
return Ok(Some(dl.binary(name)?));
}
if installation_allowed {
PBAR.step(step, &format!("Getting {}...", name));
}
match cache.download(installation_allowed, name, &[name], &url)? {
Some(dl) => Ok(Some(dl.binary(name)?)),
None => Ok(None),
}
}
/// Get the path to an existing `chromedriver`, or install it if no existing
/// binary is found.
pub fn get_or_install_chromedriver(
cache: &Cache,
mode: BuildMode,
step: &Step,
) -> Result<PathBuf, failure::Error> {
if let Ok(path) = which::which("chromedriver") {
return Ok(path);
@ -19,13 +41,14 @@ pub fn get_or_install_chromedriver(
BuildMode::Noinstall => false,
_ => true,
};
install_chromedriver(cache, installation_allowed)
install_chromedriver(cache, installation_allowed, step)
}
/// Download and install a pre-built `chromedriver` binary.
pub fn install_chromedriver(
cache: &Cache,
installation_allowed: bool,
step: &Step,
) -> Result<PathBuf, failure::Error> {
let target = if target::LINUX && target::x86_64 {
"linux64"
@ -41,13 +64,8 @@ pub fn install_chromedriver(
"https://chromedriver.storage.googleapis.com/2.46/chromedriver_{}.zip",
target
);
match cache.download(
installation_allowed,
"chromedriver",
&["chromedriver"],
&url,
)? {
Some(dl) => Ok(dl.binary("chromedriver")?),
match get_and_notify(cache, installation_allowed, "chromedriver", &url, step)? {
Some(path) => Ok(path),
None => bail!(
"No cached `chromedriver` binary found, and could not find a global \
`chromedriver` on the `$PATH`. Not installing `chromedriver` because of noinstall \
@ -61,6 +79,7 @@ pub fn install_chromedriver(
pub fn get_or_install_geckodriver(
cache: &Cache,
mode: BuildMode,
step: &Step,
) -> Result<PathBuf, failure::Error> {
if let Ok(path) = which::which("geckodriver") {
return Ok(path);
@ -69,13 +88,14 @@ pub fn get_or_install_geckodriver(
BuildMode::Noinstall => false,
_ => true,
};
install_geckodriver(cache, installation_allowed)
install_geckodriver(cache, installation_allowed, step)
}
/// Download and install a pre-built `geckodriver` binary.
pub fn install_geckodriver(
cache: &Cache,
installation_allowed: bool,
step: &Step,
) -> Result<PathBuf, failure::Error> {
let (target, ext) = if target::LINUX && target::x86 {
("linux32", "tar.gz")
@ -96,8 +116,8 @@ pub fn install_geckodriver(
target,
ext,
);
match cache.download(installation_allowed, "geckodriver", &["geckodriver"], &url)? {
Some(dl) => Ok(dl.binary("geckodriver")?),
match get_and_notify(cache, installation_allowed, "geckodriver", &url, step)? {
Some(path) => Ok(path),
None => bail!(
"No cached `geckodriver` binary found, and could not find a global `geckodriver` \
on the `$PATH`. Not installing `geckodriver` because of noinstall mode."

Loading…
Cancel
Save