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.

src/bindgen.rs

@@ -178,11 +178,7 @@ pub fn wasm_bindgen_build(
     disable_dts: bool,
     target: &str,
     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",

src/build.rs

@@ -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,6 +50,18 @@ 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 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");
@@ -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 {

src/child.rs

@@ -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
+        )
     }
 }

src/command/build.rs

@@ -265,17 +265,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(())
     }
@@ -287,9 +287,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 {:#?}.",
@@ -302,21 +302,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 {:#?}.",
@@ -325,16 +323,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(())
     }
@@ -356,7 +354,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,
@@ -365,7 +363,6 @@ impl Build {
             self.disable_dts,
             &self.target,
             self.profile,
-            step,
         )?;
         info!("wasm bindings were built at {:#?}.", &self.out_dir);
         Ok(())