From a7fe78098580baeb6af09e85a9f2d114982de9bc Mon Sep 17 00:00:00 2001 From: eblocha Date: Sat, 19 Aug 2023 12:46:09 -0400 Subject: [PATCH 1/7] Add progress bar to install command --- .changeset/show-download-progress.md | 5 ++ Cargo.lock | 73 +++++++++++++++++++++++++++- Cargo.toml | 1 + docs/commands.md | 3 ++ src/commands/install.rs | 12 +++++ src/downloader.rs | 17 ++++--- src/main.rs | 1 + src/progress.rs | 55 +++++++++++++++++++++ 8 files changed, 159 insertions(+), 8 deletions(-) create mode 100644 .changeset/show-download-progress.md create mode 100644 src/progress.rs diff --git a/.changeset/show-download-progress.md b/.changeset/show-download-progress.md new file mode 100644 index 000000000..552f42391 --- /dev/null +++ b/.changeset/show-download-progress.md @@ -0,0 +1,5 @@ +--- +"fnm": minor +--- + +Show a progress bar when downloading and extracting node diff --git a/Cargo.lock b/Cargo.lock index c8ad86ed9..aadeb14b0 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -357,6 +357,19 @@ dependencies = [ "windows-sys 0.48.0", ] +[[package]] +name = "console" +version = "0.15.7" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "c926e00cc70edefdc64d3a5ff31cc65bb97a3460097762bd23afb4d8145fccf8" +dependencies = [ + "encode_unicode", + "lazy_static", + "libc", + "unicode-width", + "windows-sys 0.45.0", +] + [[package]] name = "constant_time_eq" version = "0.1.5" @@ -540,6 +553,12 @@ dependencies = [ "winreg", ] +[[package]] +name = "encode_unicode" +version = "0.3.6" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "a357d28ed41a50f9c765dbfe56cbc04a64e53e5fc58ba79fbc34c10ef3df831f" + [[package]] name = "encoding_rs" version = "0.8.32" @@ -638,6 +657,7 @@ dependencies = [ "embed-resource", "encoding_rs_io", "env_logger", + "indicatif", "indoc", "junction", "log", @@ -912,6 +932,19 @@ dependencies = [ "hashbrown", ] +[[package]] +name = "indicatif" +version = "0.17.6" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "0b297dc40733f23a0e52728a58fa9489a5b7638a324932de16b41adc3ef80730" +dependencies = [ + "console", + "instant", + "number_prefix", + "portable-atomic", + "unicode-width", +] + [[package]] name = "indoc" version = "2.0.2" @@ -1164,6 +1197,12 @@ dependencies = [ "libc", ] +[[package]] +name = "number_prefix" +version = "0.4.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "830b246a0e5f20af87141b25c173cd1b609bd7779a4617d6ec582abaf90870f3" + [[package]] name = "object" version = "0.30.4" @@ -1248,6 +1287,12 @@ version = "0.3.27" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "26072860ba924cbfa98ea39c8c19b4dd6a4a25423dbdf219c1eca91aa0cf6964" +[[package]] +name = "portable-atomic" +version = "1.4.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "f32154ba0af3a075eefa1eda8bb414ee928f62303a54ea85b8d6638ff1a6ee9e" + [[package]] name = "pretty_assertions" version = "1.4.0" @@ -2112,7 +2157,7 @@ version = "0.48.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "e686886bc078bc1b0b600cac0147aadb815089b6e4da64016cbd754b6342700f" dependencies = [ - "windows-targets", + "windows-targets 0.48.1", ] [[package]] @@ -2130,13 +2175,37 @@ dependencies = [ "windows_x86_64_msvc 0.42.2", ] +[[package]] +name = "windows-sys" +version = "0.45.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "75283be5efb2831d37ea142365f009c02ec203cd29a3ebecbc093d52315b66d0" +dependencies = [ + "windows-targets 0.42.2", +] + [[package]] name = "windows-sys" version = "0.48.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "677d2418bec65e3338edb076e806bc1ec15693c5d0104683f2efe857f61056a9" dependencies = [ - "windows-targets", + "windows-targets 0.48.1", +] + +[[package]] +name = "windows-targets" +version = "0.42.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "8e5180c00cd44c9b1c88adb3693291f1cd93605ded80c250a75d472756b4d071" +dependencies = [ + "windows_aarch64_gnullvm 0.42.2", + "windows_aarch64_msvc 0.42.2", + "windows_i686_gnu 0.42.2", + "windows_i686_msvc 0.42.2", + "windows_x86_64_gnu 0.42.2", + "windows_x86_64_gnullvm 0.42.2", + "windows_x86_64_msvc 0.42.2", ] [[package]] diff --git a/Cargo.toml b/Cargo.toml index b9c3a135e..7b41d7d74 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -30,6 +30,7 @@ sysinfo = "0.29.3" thiserror = "1.0.44" clap_complete = "4.3.1" anyhow = "1.0.71" +indicatif = "0.17.6" [dev-dependencies] pretty_assertions = "1.4.0" diff --git a/docs/commands.md b/docs/commands.md index 00a98d3de..d3072aacc 100644 --- a/docs/commands.md +++ b/docs/commands.md @@ -216,6 +216,9 @@ Options: --latest Install latest version + --no-progress + Do not display a progress bar + --log-level The log level of fnm commands diff --git a/src/commands/install.rs b/src/commands/install.rs index 2c553844b..6b3e4aa5b 100644 --- a/src/commands/install.rs +++ b/src/commands/install.rs @@ -25,6 +25,10 @@ pub struct Install { /// Install latest version #[clap(long, conflicts_with_all = &["version", "lts"])] pub latest: bool, + + /// Do not display a progress bar + #[clap(long)] + pub no_progress: bool, } impl Install { @@ -34,16 +38,19 @@ impl Install { version: v, lts: false, latest: false, + no_progress: _, } => Ok(v), Self { version: None, lts: true, latest: false, + no_progress: _, } => Ok(Some(UserVersion::Full(Version::Lts(LtsType::Latest)))), Self { version: None, lts: false, latest: true, + no_progress: _, } => Ok(Some(UserVersion::Full(Version::Latest))), _ => Err(Error::TooManyVersionsProvided), } @@ -56,6 +63,8 @@ impl Command for Install { fn apply(self, config: &FnmConfig) -> Result<(), Self::Error> { let current_dir = std::env::current_dir().unwrap(); + let show_progress = !self.no_progress; + let current_version = self .version()? .or_else(|| get_user_version_for_directory(current_dir, config)) @@ -131,6 +140,7 @@ impl Command for Install { &config.node_dist_mirror, config.installations_dir(), safe_arch, + show_progress, ) { Err(err @ DownloaderError::VersionAlreadyInstalled { .. }) => { outln!(config, Error, "{} {}", "warning:".bold().yellow(), err); @@ -225,6 +235,7 @@ mod tests { version: UserVersion::from_str("12.0.0").ok(), lts: false, latest: false, + no_progress: true, } .apply(&config) .expect("Can't install"); @@ -250,6 +261,7 @@ mod tests { version: None, lts: false, latest: true, + no_progress: true, } .apply(&config) .expect("Can't install"); diff --git a/src/downloader.rs b/src/downloader.rs index 3e470390d..3684ab1b1 100644 --- a/src/downloader.rs +++ b/src/downloader.rs @@ -2,8 +2,10 @@ use crate::arch::Arch; use crate::archive; use crate::archive::{Error as ExtractError, Extract}; use crate::directory_portal::DirectoryPortal; +use crate::progress::ResponseProgress; use crate::version::Version; use log::debug; +use std::io::Read; use std::path::Path; use std::path::PathBuf; use thiserror::Error; @@ -63,10 +65,7 @@ fn download_url(base_url: &Url, version: &Version, arch: &Arch) -> Url { .unwrap() } -pub fn extract_archive_into>( - path: P, - response: crate::http::Response, -) -> Result<(), Error> { +fn extract_archive_into, R: Read>(path: P, response: R) -> Result<(), Error> { #[cfg(unix)] let extractor = archive::TarXz::new(response); #[cfg(windows)] @@ -81,6 +80,7 @@ pub fn install_node_dist>( node_dist_mirror: &Url, installations_dir: P, arch: &Arch, + show_progress: bool, ) -> Result<(), Error> { let installation_dir = PathBuf::from(installations_dir.as_ref()).join(version.v_str()); @@ -109,7 +109,11 @@ pub fn install_node_dist>( } debug!("Extracting response..."); - extract_archive_into(&portal, response)?; + if show_progress { + extract_archive_into(&portal, ResponseProgress::new(response))?; + } else { + extract_archive_into(&portal, response)?; + } debug!("Extraction completed"); let installed_directory = std::fs::read_dir(&portal)? @@ -171,7 +175,8 @@ mod tests { let version = Version::parse("12.0.0").unwrap(); let arch = Arch::X64; let node_dist_mirror = Url::parse("https://nodejs.org/dist/").unwrap(); - install_node_dist(&version, &node_dist_mirror, path, &arch).expect("Can't install Node 12"); + install_node_dist(&version, &node_dist_mirror, path, &arch, false) + .expect("Can't install Node 12"); let mut location_path = path.join(version.v_str()).join("installation"); diff --git a/src/main.rs b/src/main.rs index cbc4f0fb3..cfb2a64d7 100644 --- a/src/main.rs +++ b/src/main.rs @@ -22,6 +22,7 @@ mod installed_versions; mod lts; mod package_json; mod path_ext; +mod progress; mod remote_node_index; mod shell; mod system_info; diff --git a/src/progress.rs b/src/progress.rs new file mode 100644 index 000000000..54605ca41 --- /dev/null +++ b/src/progress.rs @@ -0,0 +1,55 @@ +use std::io::Read; + +use indicatif::{ProgressBar, ProgressStyle}; +use reqwest::blocking::Response; + +pub struct ResponseProgress { + progress: Option, + response: Response, +} + +fn make_progress_bar(size: u64) -> ProgressBar { + let bar = ProgressBar::new(size); + + bar.set_style( + ProgressStyle::default_bar() + .template("[{elapsed_precise}] [{bar:40}] {bytes}/{total_bytes} ({bytes_per_sec}, {eta})") + .unwrap() + .progress_chars("#>-") + ); + + bar +} + +impl ResponseProgress { + pub fn new(response: Response) -> Self { + Self { + progress: response.content_length().map(make_progress_bar), + response, + } + } + + pub fn finish(&self) { + if let Some(ref bar) = self.progress { + bar.finish(); + } + } +} + +impl Read for ResponseProgress { + fn read(&mut self, buf: &mut [u8]) -> std::io::Result { + let size = self.response.read(buf)?; + + if let Some(ref bar) = self.progress { + bar.inc(size as u64); + } + + Ok(size) + } +} + +impl Drop for ResponseProgress { + fn drop(&mut self) { + self.finish() + } +} From 9ae6a3681295c5d2b75c6637c461eeb7173bffd7 Mon Sep 17 00:00:00 2001 From: eblocha Date: Sat, 19 Aug 2023 15:38:44 -0400 Subject: [PATCH 2/7] Create progress bar using with_template to avoid double-parsing --- src/progress.rs | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/src/progress.rs b/src/progress.rs index 54605ca41..bad5b142b 100644 --- a/src/progress.rs +++ b/src/progress.rs @@ -12,10 +12,11 @@ fn make_progress_bar(size: u64) -> ProgressBar { let bar = ProgressBar::new(size); bar.set_style( - ProgressStyle::default_bar() - .template("[{elapsed_precise}] [{bar:40}] {bytes}/{total_bytes} ({bytes_per_sec}, {eta})") - .unwrap() - .progress_chars("#>-") + ProgressStyle::with_template( + "[{elapsed_precise}] [{bar:40}] {bytes}/{total_bytes} ({bytes_per_sec}, {eta})", + ) + .unwrap() + .progress_chars("#>-"), ); bar From 0298a47c1563a2af8ed244eb4f6a193dfe0bdd44 Mon Sep 17 00:00:00 2001 From: eblocha Date: Sat, 19 Aug 2023 16:03:56 -0400 Subject: [PATCH 3/7] Only show progress bar when log level is info or above --- src/commands/install.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/commands/install.rs b/src/commands/install.rs index 6b3e4aa5b..ed20fe997 100644 --- a/src/commands/install.rs +++ b/src/commands/install.rs @@ -3,6 +3,7 @@ use crate::alias::create_alias; use crate::arch::get_safe_arch; use crate::config::FnmConfig; use crate::downloader::{install_node_dist, Error as DownloaderError}; +use crate::log_level::LogLevel; use crate::lts::LtsType; use crate::outln; use crate::remote_node_index; @@ -63,7 +64,7 @@ impl Command for Install { fn apply(self, config: &FnmConfig) -> Result<(), Self::Error> { let current_dir = std::env::current_dir().unwrap(); - let show_progress = !self.no_progress; + let show_progress = !self.no_progress && config.log_level().is_writable(&LogLevel::Info); let current_version = self .version()? From d0695be4b4a701d85994560b6901e90912cfa869 Mon Sep 17 00:00:00 2001 From: eblocha Date: Sun, 20 Aug 2023 09:11:05 -0400 Subject: [PATCH 4/7] Add semicolon for clippy --- src/progress.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/progress.rs b/src/progress.rs index bad5b142b..15bdff160 100644 --- a/src/progress.rs +++ b/src/progress.rs @@ -51,6 +51,6 @@ impl Read for ResponseProgress { impl Drop for ResponseProgress { fn drop(&mut self) { - self.finish() + self.finish(); } } From 22d2dd9b6e0895f219d713ba512698ba628f79e7 Mon Sep 17 00:00:00 2001 From: eblocha Date: Mon, 21 Aug 2023 19:33:38 -0400 Subject: [PATCH 5/7] Use impl notation for extract_archive_into for readability --- src/downloader.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/downloader.rs b/src/downloader.rs index 3684ab1b1..67acdc65f 100644 --- a/src/downloader.rs +++ b/src/downloader.rs @@ -65,7 +65,7 @@ fn download_url(base_url: &Url, version: &Version, arch: &Arch) -> Url { .unwrap() } -fn extract_archive_into, R: Read>(path: P, response: R) -> Result<(), Error> { +fn extract_archive_into(path: impl AsRef, response: impl Read) -> Result<(), Error> { #[cfg(unix)] let extractor = archive::TarXz::new(response); #[cfg(windows)] From eb36bd3581e99b5a4ccd4385274ab62eea47863b Mon Sep 17 00:00:00 2001 From: eblocha Date: Mon, 21 Aug 2023 20:42:43 -0400 Subject: [PATCH 6/7] Add unit test for the progress bar. Ensures data is still read from the request. indicatif does not expose their Term struct, so it is difficult to mock a terminal to verify the progress bar looks correct. --- Cargo.lock | 1 + Cargo.toml | 1 + src/downloader.rs | 3 ++- src/progress.rs | 40 +++++++++++++++++++++++++++++++++++----- 4 files changed, 39 insertions(+), 6 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index aadeb14b0..fbc969f53 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -657,6 +657,7 @@ dependencies = [ "embed-resource", "encoding_rs_io", "env_logger", + "http", "indicatif", "indoc", "junction", diff --git a/Cargo.toml b/Cargo.toml index 7b41d7d74..f6f9326c9 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -36,6 +36,7 @@ indicatif = "0.17.6" pretty_assertions = "1.4.0" duct = "0.13.6" test-log = "0.2.12" +http = "0.2.9" [build-dependencies] embed-resource = "1.8.0" diff --git a/src/downloader.rs b/src/downloader.rs index 67acdc65f..0fcd2101b 100644 --- a/src/downloader.rs +++ b/src/downloader.rs @@ -4,6 +4,7 @@ use crate::archive::{Error as ExtractError, Extract}; use crate::directory_portal::DirectoryPortal; use crate::progress::ResponseProgress; use crate::version::Version; +use indicatif::ProgressDrawTarget; use log::debug; use std::io::Read; use std::path::Path; @@ -110,7 +111,7 @@ pub fn install_node_dist>( debug!("Extracting response..."); if show_progress { - extract_archive_into(&portal, ResponseProgress::new(response))?; + extract_archive_into(&portal, ResponseProgress::new(response, ProgressDrawTarget::stderr()))?; } else { extract_archive_into(&portal, response)?; } diff --git a/src/progress.rs b/src/progress.rs index 15bdff160..7a07c66c2 100644 --- a/src/progress.rs +++ b/src/progress.rs @@ -1,6 +1,6 @@ use std::io::Read; -use indicatif::{ProgressBar, ProgressStyle}; +use indicatif::{ProgressBar, ProgressDrawTarget, ProgressStyle}; use reqwest::blocking::Response; pub struct ResponseProgress { @@ -8,8 +8,8 @@ pub struct ResponseProgress { response: Response, } -fn make_progress_bar(size: u64) -> ProgressBar { - let bar = ProgressBar::new(size); +fn make_progress_bar(size: u64, target: ProgressDrawTarget) -> ProgressBar { + let bar = ProgressBar::with_draw_target(Some(size), target); bar.set_style( ProgressStyle::with_template( @@ -23,9 +23,11 @@ fn make_progress_bar(size: u64) -> ProgressBar { } impl ResponseProgress { - pub fn new(response: Response) -> Self { + pub fn new(response: Response, target: ProgressDrawTarget) -> Self { Self { - progress: response.content_length().map(make_progress_bar), + progress: response + .content_length() + .map(|len| make_progress_bar(len, target)), response, } } @@ -54,3 +56,31 @@ impl Drop for ResponseProgress { self.finish(); } } + +#[cfg(test)] +mod tests { + use indicatif::ProgressDrawTarget; + use reqwest::blocking::Response; + use std::io::Read; + + use super::ResponseProgress; + + const CONTENT_LENGTH: usize = 100; + + #[test] + fn test_reads_data() { + let response: Response = http::Response::builder() + .header("Content-Length", CONTENT_LENGTH) + .body("a".repeat(CONTENT_LENGTH)) + .unwrap() + .into(); + + let mut buf = [0; CONTENT_LENGTH]; + + let mut progress = ResponseProgress::new(response, ProgressDrawTarget::hidden()); + let size = progress.read(&mut buf[..]).unwrap(); + + assert_eq!(size, CONTENT_LENGTH); + assert_eq!(buf, "a".repeat(CONTENT_LENGTH).as_bytes()); + } +} From 22d764573420ef0675106ac277ee2925b6cb7976 Mon Sep 17 00:00:00 2001 From: eblocha Date: Mon, 21 Aug 2023 21:26:53 -0400 Subject: [PATCH 7/7] Add a mocked terminal that writes to a shared buffer to test the progress bar --- src/downloader.rs | 5 +++- src/progress.rs | 69 ++++++++++++++++++++++++++++++++++++++++++++--- 2 files changed, 69 insertions(+), 5 deletions(-) diff --git a/src/downloader.rs b/src/downloader.rs index 0fcd2101b..a020f57e0 100644 --- a/src/downloader.rs +++ b/src/downloader.rs @@ -111,7 +111,10 @@ pub fn install_node_dist>( debug!("Extracting response..."); if show_progress { - extract_archive_into(&portal, ResponseProgress::new(response, ProgressDrawTarget::stderr()))?; + extract_archive_into( + &portal, + ResponseProgress::new(response, ProgressDrawTarget::stderr()), + )?; } else { extract_archive_into(&portal, response)?; } diff --git a/src/progress.rs b/src/progress.rs index 7a07c66c2..ec8e301e8 100644 --- a/src/progress.rs +++ b/src/progress.rs @@ -59,16 +59,64 @@ impl Drop for ResponseProgress { #[cfg(test)] mod tests { - use indicatif::ProgressDrawTarget; + use indicatif::{ProgressDrawTarget, TermLike}; use reqwest::blocking::Response; - use std::io::Read; + use std::{ + io::Read, + sync::{Arc, Mutex}, + }; use super::ResponseProgress; const CONTENT_LENGTH: usize = 100; + #[derive(Debug)] + struct MockedTerm { + pub buf: Arc>, + } + + impl TermLike for MockedTerm { + fn width(&self) -> u16 { + 80 + } + + fn move_cursor_up(&self, _n: usize) -> std::io::Result<()> { + Ok(()) + } + + fn move_cursor_down(&self, _n: usize) -> std::io::Result<()> { + Ok(()) + } + + fn move_cursor_right(&self, _n: usize) -> std::io::Result<()> { + Ok(()) + } + + fn move_cursor_left(&self, _n: usize) -> std::io::Result<()> { + Ok(()) + } + + fn write_line(&self, s: &str) -> std::io::Result<()> { + self.buf.lock().unwrap().push_str(s); + Ok(()) + } + + fn write_str(&self, s: &str) -> std::io::Result<()> { + self.buf.lock().unwrap().push_str(s); + Ok(()) + } + + fn clear_line(&self) -> std::io::Result<()> { + Ok(()) + } + + fn flush(&self) -> std::io::Result<()> { + Ok(()) + } + } + #[test] - fn test_reads_data() { + fn test_reads_data_and_shows_progress() { let response: Response = http::Response::builder() .header("Content-Length", CONTENT_LENGTH) .body("a".repeat(CONTENT_LENGTH)) @@ -77,10 +125,23 @@ mod tests { let mut buf = [0; CONTENT_LENGTH]; - let mut progress = ResponseProgress::new(response, ProgressDrawTarget::hidden()); + let out_buf = Arc::new(Mutex::new(String::new())); + + let mut progress = ResponseProgress::new( + response, + ProgressDrawTarget::term_like(Box::new(MockedTerm { + buf: out_buf.clone(), + })), + ); let size = progress.read(&mut buf[..]).unwrap(); + drop(progress); + assert_eq!(size, CONTENT_LENGTH); assert_eq!(buf, "a".repeat(CONTENT_LENGTH).as_bytes()); + assert!(out_buf + .lock() + .unwrap() + .contains(&format!("[{}]", &"#".repeat(40)))); } }