From b710fde193ca4c1335bbec61df976ba52222ddbf Mon Sep 17 00:00:00 2001 From: Alex Crichton Date: Fri, 1 Mar 2019 09:35:45 -0800 Subject: [PATCH] Remove the no longer needed `Step` type While historically used to count steps in stages the count is no longer present in the output so this should no longer be necessary. --- src/bindgen.rs | 4 +--- src/build.rs | 8 +++----- src/command/build.rs | 34 +++++++++++++++------------------- src/command/test.rs | 35 +++++++++++++++-------------------- src/progressbar.rs | 32 ++------------------------------ src/test/webdriver.rs | 16 +++++----------- tests/all/utils/fixture.rs | 10 ++++------ tests/all/webdriver.rs | 6 ++---- 8 files changed, 47 insertions(+), 98 deletions(-) diff --git a/src/bindgen.rs b/src/bindgen.rs index 58c5d85..4b3a629 100644 --- a/src/bindgen.rs +++ b/src/bindgen.rs @@ -8,7 +8,6 @@ use failure::{self, ResultExt}; use log::debug; use log::{info, warn}; use manifest::CrateData; -use progressbar::Step; use std::env; use std::fs; use std::path::{Path, PathBuf}; @@ -27,7 +26,6 @@ pub fn install_wasm_bindgen( cache: &Cache, version: &str, install_permitted: bool, - step: &Step, ) -> Result { // If `wasm-bindgen` is installed globally and it has the right version, use // that. Assume that other tools are installed next to it. @@ -42,7 +40,7 @@ pub fn install_wasm_bindgen( } let msg = format!("{}Installing wasm-bindgen...", emoji::DOWN_ARROW); - PBAR.step(step, &msg); + PBAR.step(&msg); let dl = download_prebuilt_wasm_bindgen(&cache, version, install_permitted); match dl { diff --git a/src/build.rs b/src/build.rs index ef0f159..ab34543 100644 --- a/src/build.rs +++ b/src/build.rs @@ -4,7 +4,6 @@ use child; use command::build::BuildProfile; use emoji; use failure::{Error, ResultExt}; -use progressbar::Step; use std::path::Path; use std::process::Command; use std::str; @@ -49,7 +48,7 @@ fn rustc_minor_version() -> Option { /// Ensure that `rustup` has the `wasm32-unknown-unknown` target installed for /// current toolchain -pub fn rustup_add_wasm_target(step: &Step) -> Result<(), Error> { +pub fn rustup_add_wasm_target() -> Result<(), Error> { let mut cmd = Command::new("rustc"); cmd.arg("--print").arg("sysroot"); let output = @@ -64,7 +63,7 @@ pub fn rustup_add_wasm_target(step: &Step) -> Result<(), Error> { // ... otherwise fall back to rustup to add the target let msg = format!("{}Adding Wasm target...", emoji::TARGET); - PBAR.step(step, &msg); + PBAR.step(&msg); let mut cmd = Command::new("rustup"); cmd.arg("target").arg("add").arg("wasm32-unknown-unknown"); child::run(cmd, "rustup").context("Adding the wasm32-unknown-unknown target with rustup")?; @@ -75,11 +74,10 @@ pub fn rustup_add_wasm_target(step: &Step) -> Result<(), Error> { pub fn cargo_build_wasm( path: &Path, profile: BuildProfile, - step: &Step, extra_options: &Vec, ) -> Result<(), Error> { let msg = format!("{}Compiling to Wasm...", emoji::CYCLONE); - PBAR.step(step, &msg); + PBAR.step(&msg); let mut cmd = Command::new("cargo"); cmd.current_dir(path).arg("build").arg("--lib"); match profile { diff --git a/src/command/build.rs b/src/command/build.rs index 5dabf3d..2fc1d41 100644 --- a/src/command/build.rs +++ b/src/command/build.rs @@ -11,7 +11,6 @@ use license; use lockfile::Lockfile; use log::info; use manifest; -use progressbar::Step; use readme; use std::path::PathBuf; use std::str::FromStr; @@ -183,7 +182,7 @@ impl Default for BuildOptions { } } -type BuildStep = fn(&mut Build, &Step) -> Result<(), Error>; +type BuildStep = fn(&mut Build) -> Result<(), Error>; impl Build { /// Construct a build command from the given options. @@ -226,13 +225,10 @@ impl Build { pub fn run(&mut self) -> Result<(), Error> { let process_steps = Build::get_process_steps(self.mode); - let mut step_counter = Step::new(process_steps.len()); - let started = Instant::now(); for (_, process_step) in process_steps { - process_step(self, &step_counter)?; - step_counter.inc(); + process_step(self)?; } let duration = crate::command::utils::elapsed(started.elapsed()); @@ -297,7 +293,7 @@ impl Build { } } - fn step_check_rustc_version(&mut self, _step: &Step) -> Result<(), Error> { + fn step_check_rustc_version(&mut self) -> Result<(), Error> { info!("Checking rustc version..."); let version = build::check_rustc_version()?; let msg = format!("rustc version is {}.", version); @@ -305,23 +301,23 @@ impl Build { Ok(()) } - fn step_check_crate_config(&mut self, _step: &Step) -> Result<(), Error> { + fn step_check_crate_config(&mut self) -> Result<(), Error> { info!("Checking crate configuration..."); self.crate_data.check_crate_config()?; info!("Crate is correctly configured."); Ok(()) } - fn step_add_wasm_target(&mut self, step: &Step) -> Result<(), Error> { + fn step_add_wasm_target(&mut self) -> Result<(), Error> { info!("Adding wasm-target..."); - build::rustup_add_wasm_target(step)?; + build::rustup_add_wasm_target()?; info!("Adding wasm-target was successful."); Ok(()) } - fn step_build_wasm(&mut self, step: &Step) -> Result<(), Error> { + fn step_build_wasm(&mut self) -> 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,14 +330,14 @@ impl Build { Ok(()) } - fn step_create_dir(&mut self, _step: &Step) -> Result<(), Error> { + fn step_create_dir(&mut self) -> Result<(), Error> { info!("Creating a pkg directory..."); 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> { + fn step_create_json(&mut self) -> Result<(), Error> { self.crate_data.write_package_json( &self.out_dir, &self.scope, @@ -355,21 +351,21 @@ impl Build { Ok(()) } - fn step_copy_readme(&mut self, _step: &Step) -> Result<(), Error> { + fn step_copy_readme(&mut self) -> Result<(), Error> { info!("Copying readme from crate..."); 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) -> Result<(), failure::Error> { info!("Copying license from crate..."); license::copy_from_crate(&self.crate_data, &self.crate_path, &self.out_dir)?; info!("Copied license from crate to {:#?}.", &self.out_dir); Ok(()) } - fn step_install_wasm_bindgen(&mut self, step: &Step) -> Result<(), failure::Error> { + fn step_install_wasm_bindgen(&mut self) -> Result<(), failure::Error> { info!("Identifying wasm-bindgen dependency..."); let lockfile = Lockfile::new(&self.crate_data)?; let bindgen_version = lockfile.require_wasm_bindgen()?; @@ -380,13 +376,13 @@ impl Build { BuildMode::Noinstall => false, }; let bindgen = - bindgen::install_wasm_bindgen(&self.cache, &bindgen_version, install_permitted, step)?; + bindgen::install_wasm_bindgen(&self.cache, &bindgen_version, install_permitted)?; self.bindgen = Some(bindgen); info!("Installing wasm-bindgen-cli was successful."); Ok(()) } - fn step_run_wasm_bindgen(&mut self, _step: &Step) -> Result<(), Error> { + fn step_run_wasm_bindgen(&mut self) -> Result<(), Error> { info!("Building the wasm bindings..."); bindgen::wasm_bindgen_build( &self.crate_data, diff --git a/src/command/test.rs b/src/command/test.rs index 85f945c..da7cf29 100644 --- a/src/command/test.rs +++ b/src/command/test.rs @@ -11,7 +11,6 @@ use failure::Error; use lockfile::Lockfile; use log::info; use manifest; -use progressbar::Step; use std::path::PathBuf; use std::time::Instant; use test::{self, webdriver}; @@ -100,7 +99,7 @@ pub struct Test { extra_options: Vec, } -type TestStep = fn(&mut Test, &Step) -> Result<(), Error>; +type TestStep = fn(&mut Test) -> Result<(), Error>; impl Test { /// Construct a test command from the given options. @@ -162,12 +161,10 @@ impl Test { /// Execute this test command. pub fn run(mut self) -> Result<(), Error> { let process_steps = self.get_process_steps(); - let mut step_counter = Step::new(process_steps.len()); let started = Instant::now(); for (_, process_step) in process_steps { - process_step(&mut self, &step_counter)?; - step_counter.inc(); + process_step(&mut self)?; } let duration = crate::command::utils::elapsed(started.elapsed()); info!("Done in {}.", &duration); @@ -230,21 +227,21 @@ impl Test { } } - fn step_check_rustc_version(&mut self, _step: &Step) -> Result<(), Error> { + fn step_check_rustc_version(&mut self) -> Result<(), Error> { info!("Checking rustc version..."); let _ = build::check_rustc_version()?; info!("Rustc version is correct."); Ok(()) } - fn step_add_wasm_target(&mut self, step: &Step) -> Result<(), Error> { + fn step_add_wasm_target(&mut self) -> Result<(), Error> { info!("Adding wasm-target..."); - build::rustup_add_wasm_target(step)?; + build::rustup_add_wasm_target()?; info!("Adding wasm-target was successful."); Ok(()) } - fn step_build_tests(&mut self, _step: &Step) -> Result<(), Error> { + fn step_build_tests(&mut self) -> Result<(), Error> { info!("Compiling tests to wasm..."); build::cargo_build_wasm_tests(&self.crate_path, !self.release)?; @@ -253,7 +250,7 @@ impl Test { Ok(()) } - fn step_install_wasm_bindgen(&mut self, step: &Step) -> Result<(), Error> { + fn step_install_wasm_bindgen(&mut self) -> Result<(), Error> { info!("Identifying wasm-bindgen dependency..."); let lockfile = Lockfile::new(&self.crate_data)?; let bindgen_version = lockfile.require_wasm_bindgen()?; @@ -288,7 +285,7 @@ impl Test { }; let dl = - bindgen::install_wasm_bindgen(&self.cache, &bindgen_version, install_permitted, step)?; + bindgen::install_wasm_bindgen(&self.cache, &bindgen_version, install_permitted)?; self.test_runner_path = Some(dl.binary("wasm-bindgen-test-runner")?); @@ -296,7 +293,7 @@ impl Test { Ok(()) } - fn step_test_node(&mut self, _step: &Step) -> Result<(), Error> { + fn step_test_node(&mut self) -> Result<(), Error> { assert!(self.node); info!("Running tests in node..."); test::cargo_test_wasm( @@ -312,18 +309,17 @@ impl Test { Ok(()) } - fn step_get_chromedriver(&mut self, step: &Step) -> Result<(), Error> { + fn step_get_chromedriver(&mut self) -> Result<(), Error> { 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> { + fn step_test_chrome(&mut self) -> Result<(), Error> { let chromedriver = self.chromedriver.as_ref().unwrap().display().to_string(); let chromedriver = chromedriver.as_str(); info!( @@ -352,18 +348,17 @@ impl Test { Ok(()) } - fn step_get_geckodriver(&mut self, step: &Step) -> Result<(), Error> { + fn step_get_geckodriver(&mut self) -> Result<(), Error> { 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> { + fn step_test_firefox(&mut self) -> Result<(), Error> { let geckodriver = self.geckodriver.as_ref().unwrap().display().to_string(); let geckodriver = geckodriver.as_str(); info!( @@ -392,14 +387,14 @@ impl Test { Ok(()) } - fn step_get_safaridriver(&mut self, _step: &Step) -> Result<(), Error> { + fn step_get_safaridriver(&mut self) -> 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> { + fn step_test_safari(&mut self) -> Result<(), Error> { let safaridriver = self.safaridriver.as_ref().unwrap().display().to_string(); let safaridriver = safaridriver.as_str(); info!( diff --git a/src/progressbar.rs b/src/progressbar.rs index acc1448..6d4a9aa 100644 --- a/src/progressbar.rs +++ b/src/progressbar.rs @@ -2,7 +2,6 @@ use console::style; use emoji; -use std::fmt; /// Synchronized progress bar and status message printing. pub struct ProgressOutput; @@ -10,8 +9,8 @@ pub struct ProgressOutput; impl ProgressOutput { /// Inform the user that the given `step` is being executed, with details in /// `message`. - pub fn step(&self, step: &Step, message: &str) { - let msg = format!("{} {}", style(step).bold().dim(), message); + pub fn step(&self, message: &str) { + let msg = format!("{} {}", style(emoji::INFO).bold().dim(), message); self.message(&msg) } @@ -58,33 +57,6 @@ impl ProgressOutput { } } -/// For processes that can be broken down into N fractional steps, with messages -/// added for each step along the way like -/// -/// > [2/5] Doing the second step out of five. -pub struct Step { - current: usize, - total: usize, -} - -impl Step { - /// Construct a `Step` where there are `total` number of steps. - pub fn new(total: usize) -> Step { - Step { current: 1, total } - } - - /// Increment the current step. - pub fn inc(&mut self) { - self.current += 1; - } -} - -impl fmt::Display for Step { - fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { - write!(f, "{}", emoji::INFO) - } -} - impl Default for ProgressOutput { fn default() -> Self { ProgressOutput diff --git a/src/test/webdriver.rs b/src/test/webdriver.rs index 6a645e9..75aee6f 100644 --- a/src/test/webdriver.rs +++ b/src/test/webdriver.rs @@ -1,6 +1,5 @@ //! Getting WebDriver client binaries. -use crate::progressbar::Step; use binary_install::Cache; use command::build::BuildMode; use failure; @@ -13,13 +12,12 @@ fn get_and_notify( installation_allowed: bool, name: &str, url: &str, - step: &Step, ) -> Result, 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)); + PBAR.step(&format!("Getting {}...", name)); } match cache.download(installation_allowed, name, &[name], &url)? { Some(dl) => Ok(Some(dl.binary(name)?)), @@ -32,7 +30,6 @@ fn get_and_notify( pub fn get_or_install_chromedriver( cache: &Cache, mode: BuildMode, - step: &Step, ) -> Result { if let Ok(path) = which::which("chromedriver") { return Ok(path); @@ -41,14 +38,13 @@ pub fn get_or_install_chromedriver( BuildMode::Noinstall => false, _ => true, }; - install_chromedriver(cache, installation_allowed, step) + install_chromedriver(cache, installation_allowed) } /// Download and install a pre-built `chromedriver` binary. pub fn install_chromedriver( cache: &Cache, installation_allowed: bool, - step: &Step, ) -> Result { let target = if target::LINUX && target::x86_64 { "linux64" @@ -64,7 +60,7 @@ pub fn install_chromedriver( "https://chromedriver.storage.googleapis.com/2.46/chromedriver_{}.zip", target ); - match get_and_notify(cache, installation_allowed, "chromedriver", &url, step)? { + match get_and_notify(cache, installation_allowed, "chromedriver", &url)? { Some(path) => Ok(path), None => bail!( "No cached `chromedriver` binary found, and could not find a global \ @@ -79,7 +75,6 @@ pub fn install_chromedriver( pub fn get_or_install_geckodriver( cache: &Cache, mode: BuildMode, - step: &Step, ) -> Result { if let Ok(path) = which::which("geckodriver") { return Ok(path); @@ -88,14 +83,13 @@ pub fn get_or_install_geckodriver( BuildMode::Noinstall => false, _ => true, }; - install_geckodriver(cache, installation_allowed, step) + install_geckodriver(cache, installation_allowed) } /// Download and install a pre-built `geckodriver` binary. pub fn install_geckodriver( cache: &Cache, installation_allowed: bool, - step: &Step, ) -> Result { let (target, ext) = if target::LINUX && target::x86 { ("linux32", "tar.gz") @@ -116,7 +110,7 @@ pub fn install_geckodriver( target, ext, ); - match get_and_notify(cache, installation_allowed, "geckodriver", &url, step)? { + match get_and_notify(cache, installation_allowed, "geckodriver", &url)? { Some(path) => Ok(path), None => bail!( "No cached `geckodriver` binary found, and could not find a global `geckodriver` \ diff --git a/tests/all/utils/fixture.rs b/tests/all/utils/fixture.rs index 11c323e..fced91c 100644 --- a/tests/all/utils/fixture.rs +++ b/tests/all/utils/fixture.rs @@ -245,11 +245,10 @@ impl Fixture { let cache = self.cache(); // like above for synchronization - let step = wasm_pack::progressbar::Step::new(1); FETCH_GECKODRIVER.call_once(|| { - wasm_pack::test::webdriver::install_geckodriver(&cache, true, &step).unwrap(); + wasm_pack::test::webdriver::install_geckodriver(&cache, true).unwrap(); }); - wasm_pack::test::webdriver::install_geckodriver(&cache, true, &step).unwrap() + wasm_pack::test::webdriver::install_geckodriver(&cache, true).unwrap() } /// Download `chromedriver` and return its path. @@ -261,11 +260,10 @@ impl Fixture { let cache = self.cache(); // like above for synchronization - let step = wasm_pack::progressbar::Step::new(1); FETCH_CHROMEDRIVER.call_once(|| { - wasm_pack::test::webdriver::install_chromedriver(&cache, true, &step).unwrap(); + wasm_pack::test::webdriver::install_chromedriver(&cache, true).unwrap(); }); - wasm_pack::test::webdriver::install_chromedriver(&cache, true, &step).unwrap() + wasm_pack::test::webdriver::install_chromedriver(&cache, true).unwrap() } pub fn cache_dir(&self) -> PathBuf { diff --git a/tests/all/webdriver.rs b/tests/all/webdriver.rs index 15f84e3..6223290 100644 --- a/tests/all/webdriver.rs +++ b/tests/all/webdriver.rs @@ -12,8 +12,7 @@ use wasm_pack::test::webdriver; fn can_install_chromedriver() { let fixture = fixture::js_hello_world(); let cache = Cache::at(&fixture.path); - let step = wasm_pack::progressbar::Step::new(1); - assert!(webdriver::install_chromedriver(&cache, true, &step).is_ok()); + assert!(webdriver::install_chromedriver(&cache, true).is_ok()); } #[test] @@ -27,6 +26,5 @@ fn can_install_chromedriver() { fn can_install_geckodriver() { let fixture = fixture::js_hello_world(); let cache = Cache::at(&fixture.path); - let step = wasm_pack::progressbar::Step::new(1); - assert!(webdriver::install_geckodriver(&cache, true, &step).is_ok()); + assert!(webdriver::install_geckodriver(&cache, true).is_ok()); }