From a4c651aa33591222c3c611985e0d61ca601a979c Mon Sep 17 00:00:00 2001 From: Stu Hood Date: Tue, 2 Aug 2022 10:37:33 -0400 Subject: [PATCH 1/2] Update to `indicatif` `0.17`. Spinners not working. # Building wheels and fs_util will be skipped. Delete if not intended. [ci skip-build-wheels] --- src/rust/engine/Cargo.lock | 7 +++--- src/rust/engine/ui/Cargo.toml | 2 +- src/rust/engine/ui/src/console_ui.rs | 33 ++++++++++------------------ 3 files changed, 16 insertions(+), 26 deletions(-) diff --git a/src/rust/engine/Cargo.lock b/src/rust/engine/Cargo.lock index c83236c47c1..5f0adc95bca 100644 --- a/src/rust/engine/Cargo.lock +++ b/src/rust/engine/Cargo.lock @@ -1340,14 +1340,13 @@ dependencies = [ [[package]] name = "indicatif" -version = "0.16.2" +version = "0.17.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "2d207dc617c7a380ab07ff572a6e52fa202a2a8f355860ac9c38e23f8196be1b" +checksum = "fcc42b206e70d86ec03285b123e65a5458c92027d1fb2ae3555878b8113b3ddf" dependencies = [ "console", - "lazy_static", "number_prefix", - "regex", + "unicode-width", ] [[package]] diff --git a/src/rust/engine/ui/Cargo.toml b/src/rust/engine/ui/Cargo.toml index 0857e34ceef..ec3ee6bb9e7 100644 --- a/src/rust/engine/ui/Cargo.toml +++ b/src/rust/engine/ui/Cargo.toml @@ -11,7 +11,7 @@ path = "src/console_ui.rs" console = "0.15.1" futures = "0.3" indexmap = "1.9" -indicatif = "0.16.2" +indicatif = "0.17" logging = { path = "../logging" } parking_lot = "0.12" # TODO: See https://github.com/Byron/prodash/pull/9. diff --git a/src/rust/engine/ui/src/console_ui.rs b/src/rust/engine/ui/src/console_ui.rs index 403f36a118f..c5576471ef4 100644 --- a/src/rust/engine/ui/src/console_ui.rs +++ b/src/rust/engine/ui/src/console_ui.rs @@ -25,7 +25,7 @@ // Arc can be more clear than needing to grok Orderings: #![allow(clippy::mutex_atomic)] -use futures::future::{BoxFuture, FutureExt}; +use futures::future::{self, BoxFuture, FutureExt}; use indexmap::IndexSet; use indicatif::{MultiProgress, ProgressBar, ProgressDrawTarget, ProgressStyle, WeakProgressBar}; use parking_lot::Mutex; @@ -34,7 +34,6 @@ use std::collections::HashMap; use std::collections::HashSet; use std::fmt; use std::future::Future; -use std::pin::Pin; use std::sync::mpsc; use std::sync::Arc; use std::time::{Duration, SystemTime, UNIX_EPOCH}; @@ -71,12 +70,12 @@ impl ConsoleUI { /// /// The number of times per-second that `Self::render` should be called. /// - pub fn render_rate_hz() -> u64 { + pub fn render_rate_hz() -> u8 { 10 } pub fn render_interval() -> Duration { - Duration::from_millis(1000 / Self::render_rate_hz()) + Duration::from_millis(1000 / (Self::render_rate_hz() as u64)) } pub async fn with_console_ui_disabled(&mut self, f: impl Future) -> T { @@ -142,9 +141,9 @@ impl ConsoleUI { } struct IndicatifInstance { - // NB: The indeces correspond to indeces in tasks_to_display: IndexSet, - multi_progress_task: Pin> + Send>>, + // NB: Kept for Drop. + _multi_progress: MultiProgress, bars: Vec, } @@ -246,6 +245,7 @@ impl Instance { } else { // We take exclusive access to stdio by registering a callback that is used to render stderr // while we're holding access. See the method doc. + // TODO: `MultiProgress` now supports `println` directly. let stderr_dest_bar: Arc>> = Arc::new(Mutex::new(None)); // We acquire the lock before taking exclusive access, and don't release it until after // initialization. That way, the exclusive callback can always assume that the destination @@ -279,20 +279,17 @@ impl Instance { let multi_progress = MultiProgress::with_draw_target(draw_target); let bars = (0..cmp::min(local_parallelism, terminal_height.into())) .map(|_n| { - let style = ProgressStyle::default_bar().template("{spinner} {wide_msg}"); + let style = ProgressStyle::default_bar() + .template("{spinner} {wide_msg}") + .expect("Valid template."); multi_progress.add(ProgressBar::new(terminal_width.into()).with_style(style)) }) .collect::>(); *stderr_dest_bar_guard = Some(bars[0].downgrade()); - // Spawn a task to drive the multi progress. - let multi_progress_task = executor - .spawn_blocking(move || multi_progress.join()) - .boxed(); - Ok(Instance::Indicatif(IndicatifInstance { tasks_to_display: IndexSet::new(), - multi_progress_task, + _multi_progress: multi_progress, bars, })) }; @@ -396,14 +393,8 @@ impl Instance { Instance::Indicatif(indicatif) => { // When the MultiProgress completes, the Term(Destination) is dropped, which will restore // direct access to the Console. - indicatif - .multi_progress_task - .map(|res| { - // If teardown fails, we can't know whether any error messages will even reach the client, so - // there isn't much point in trying to recover gracefully. - res.unwrap_or_else(|e| panic!("Failed to render UI: {}", e)) - }) - .boxed() + std::mem::drop(indicatif); + future::ready(()).boxed() } Instance::Prodash(mut prodash) => { // Drop all tasks to clear the Tree. The call to shutdown will render a final "Tick" with the From b08309c7a12590484dfb1c45866253a9d5be2f01 Mon Sep 17 00:00:00 2001 From: Stu Hood Date: Thu, 18 Aug 2022 14:17:45 -0700 Subject: [PATCH 2/2] Manually `tick()`. # Building wheels and fs_util will be skipped. Delete if not intended. [ci skip-build-wheels] --- src/rust/engine/Cargo.lock | 12 ++++++------ src/rust/engine/ui/src/console_ui.rs | 3 +++ 2 files changed, 9 insertions(+), 6 deletions(-) diff --git a/src/rust/engine/Cargo.lock b/src/rust/engine/Cargo.lock index 5f0adc95bca..1dfd1a3845b 100644 --- a/src/rust/engine/Cargo.lock +++ b/src/rust/engine/Cargo.lock @@ -1461,9 +1461,9 @@ checksum = "e2abad23fbc42b3700f2f279844dc832adb2b2eb069b2df918f455c4e18cc646" [[package]] name = "libc" -version = "0.2.126" +version = "0.2.132" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "349d5a591cd28b49e1d1037471617a32ddcda5731b99419008085f72d5a53836" +checksum = "8371e4e5341c3a96db127eb2465ac681ced4c433e01dd0e938adbef26ba93ba5" [[package]] name = "lmdb-rkv" @@ -1878,9 +1878,9 @@ checksum = "b8f8bdf33df195859076e54ab11ee78a1b208382d3a26ec40d142ffc1ecc49ef" [[package]] name = "once_cell" -version = "1.8.0" +version = "1.13.1" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "692fcb63b64b1758029e0a96ee63e049ce8c5948587f2f7208df04625e5f6b56" +checksum = "074864da206b4973b84eb91683020dbefd6a8c3f0f38e054d93954e891935e4e" [[package]] name = "oorandom" @@ -3572,9 +3572,9 @@ checksum = "bb0d2e7be6ae3a5fa87eed5fb451aff96f2573d2694942e40543ae0bbe19c796" [[package]] name = "unicode-width" -version = "0.1.8" +version = "0.1.9" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "9337591893a19b88d8d87f2cec1e73fad5cdfd10e5a6f349f498ad6ea2ffb1e3" +checksum = "3ed742d4ea2bd1176e236172c8429aaf54486e7ac098db29ffe6529e0ce50973" [[package]] name = "unicode-xid" diff --git a/src/rust/engine/ui/src/console_ui.rs b/src/rust/engine/ui/src/console_ui.rs index c5576471ef4..d8f488fb205 100644 --- a/src/rust/engine/ui/src/console_ui.rs +++ b/src/rust/engine/ui/src/console_ui.rs @@ -347,6 +347,9 @@ impl Instance { Some(label) => pbar.set_message(label), None => pbar.set_message(""), } + // TODO: See https://github.com/console-rs/indicatif/pull/417#issuecomment-1202773191 + // Can be removed once we upgrade past `0.17.0`. + pbar.tick(); } } Instance::Prodash(prodash) => {