diff --git a/CHANGELOG.md b/CHANGELOG.md index 8d69ef8fb3..cfbdd645a0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,13 @@ "You know what they say. Fool me once, strike one, but fool me twice... strike three." — Michael Scott +## Unreleased + +### Fixes + +- Don't use hidden progress bar ([#2830](https://github.com/getsentry/sentry-cli/pull/2830)) by @lcian + - Fixed an issue where log messages would not show up when redirecting stderr to a file and using any subcommand that uses a progress bar, such as `sentry-cli debug-files bundle-jvm` and `sentry-cli sourcemaps upload`. + ## 2.56.1 ### Deprecations diff --git a/src/commands/debug_files/find.rs b/src/commands/debug_files/find.rs index 4b63cf2517..f1e1fa907d 100644 --- a/src/commands/debug_files/find.rs +++ b/src/commands/debug_files/find.rs @@ -117,7 +117,7 @@ fn find_ids( .filter(|e| e.file_type().is_file()); let mut found_files = vec![]; - let pb = ProgressBar::new_spinner(); + let mut pb = ProgressBar::new_spinner(); pb.set_style( ProgressStyle::default_spinner() .tick_chars("/|\\- ") @@ -136,7 +136,7 @@ fn find_ids( pb.set_message(p); } pb.tick(); - pb.set_prefix(&format!("{}", found_files.len())); + pb.set_prefix(format!("{}", found_files.len())); let found: Vec<_> = types .iter() diff --git a/src/utils/chunks/upload.rs b/src/utils/chunks/upload.rs index a05c8425af..7425fe104f 100644 --- a/src/utils/chunks/upload.rs +++ b/src/utils/chunks/upload.rs @@ -182,7 +182,7 @@ where ); let api = Api::current(); - let pb = ProgressBar::new(chunked_objects.len()); + let mut pb = ProgressBar::new(chunked_objects.len()); pb.set_style(progress_style); pb.set_prefix(">"); diff --git a/src/utils/dif_upload/mod.rs b/src/utils/dif_upload/mod.rs index 1b12e2d892..9f4b6a9344 100644 --- a/src/utils/dif_upload/mod.rs +++ b/src/utils/dif_upload/mod.rs @@ -636,7 +636,7 @@ fn search_difs(options: &DifUpload) -> Result>> { \n found {prefix:.yellow} {msg:.dim}", ); - let pb = ProgressBar::new_spinner(); + let mut pb = ProgressBar::new_spinner(); pb.enable_steady_tick(100); pb.set_style(progress_style); @@ -668,7 +668,7 @@ fn search_difs(options: &DifUpload) -> Result>> { } }; - pb.set_prefix(&collected.len().to_string()); + pb.set_prefix(collected.len().to_string()); Ok(()) })?; } @@ -958,7 +958,7 @@ where \n{wide_bar} {pos}/{len}", ); - let pb = ProgressBar::new(items.len()); + let mut pb = ProgressBar::new(items.len()); pb.set_style(progress_style); pb.set_prefix(">"); @@ -1019,7 +1019,7 @@ fn process_symbol_maps<'a>( \n{wide_bar} {pos}/{len}", ); - let pb = ProgressBar::new(len); + let mut pb = ProgressBar::new(len); pb.set_style(progress_style); pb.set_prefix(">"); @@ -1110,7 +1110,7 @@ fn create_source_bundles<'a>( \n{wide_bar} {pos}/{len}", ); - let pb = ProgressBar::new(difs.len()); + let mut pb = ProgressBar::new(difs.len()); pb.set_style(progress_style); pb.set_prefix(">"); @@ -1166,7 +1166,7 @@ fn create_il2cpp_mappings<'a>(difs: &[DifMatch<'a>]) -> Result> \n{wide_bar} {pos}/{len}", ); - let pb = ProgressBar::new(difs.len()); + let mut pb = ProgressBar::new(difs.len()); pb.set_style(progress_style); pb.set_prefix(">"); diff --git a/src/utils/file_search.rs b/src/utils/file_search.rs index 539f0400e1..1fdb50fb14 100644 --- a/src/utils/file_search.rs +++ b/src/utils/file_search.rs @@ -98,7 +98,7 @@ impl ReleaseFileSearch { \n found {prefix:.yellow} {msg:.dim}", ); - let pb = ProgressBar::new_spinner(); + let mut pb = ProgressBar::new_spinner(); pb.enable_steady_tick(100); pb.set_style(progress_style); @@ -139,7 +139,7 @@ impl ReleaseFileSearch { if file.file_type().is_some_and(|t| t.is_dir()) { continue; } - pb.set_message(&format!("{}", file.path().display())); + pb.set_message(format!("{}", file.path().display())); info!("found: {} ({} bytes)", file.path().display(), { #[expect(clippy::unwrap_used, reason = "legacy code")] @@ -164,7 +164,7 @@ impl ReleaseFileSearch { }; collected.push(file_match); - pb.set_prefix(&collected.len().to_string()); + pb.set_prefix(collected.len().to_string()); } pb.finish_and_clear(); diff --git a/src/utils/file_upload.rs b/src/utils/file_upload.rs index 15db8419a9..78714914a4 100644 --- a/src/utils/file_upload.rs +++ b/src/utils/file_upload.rs @@ -684,7 +684,7 @@ fn build_artifact_bundle( \n{wide_bar} {pos}/{len}", ); - let pb = ProgressBar::new(files.len()); + let mut pb = ProgressBar::new(files.len()); pb.set_style(progress_style); pb.set_prefix(">"); diff --git a/src/utils/progress.rs b/src/utils/progress.rs index d0355c618f..cc912fd5b7 100644 --- a/src/utils/progress.rs +++ b/src/utils/progress.rs @@ -1,6 +1,6 @@ use parking_lot::RwLock; use std::env; -use std::ops::Deref; +use std::io::IsTerminal as _; use std::sync::Arc; use std::time::Instant; @@ -8,69 +8,134 @@ use crate::utils::logging; pub use indicatif::ProgressStyle; -pub fn is_progress_bar_visible() -> bool { - env::var("SENTRY_NO_PROGRESS_BAR") != Ok("1".into()) +/// By default, use the progress bar when we can detect we're running in a terminal. +/// `SENTRY_NO_PROGRESS_BAR` takes precedence and can be used to disable the progress bar +/// regardless of whether or not we're in a terminal. +pub fn use_progress_bar() -> bool { + if env::var("SENTRY_NO_PROGRESS_BAR") == Ok("1".into()) { + return false; + } + std::io::stderr().is_terminal() } +/// Wrapper that optionally holds a progress bar. +/// If there's a progress bar, forward calls to it. +/// Otherwise, forward messages to `log` calls. pub struct ProgressBar { - inner: Arc, + inner: Option>, start: Instant, + prefix: Option, } impl ProgressBar { pub fn new(len: usize) -> Self { - if is_progress_bar_visible() { - indicatif::ProgressBar::new(len as u64).into() + if use_progress_bar() { + Self::from_indicatif(indicatif::ProgressBar::new(len as u64)) } else { - Self::hidden() + Self::no_progress_bar() } } pub fn new_spinner() -> Self { - if is_progress_bar_visible() { - indicatif::ProgressBar::new_spinner().into() + if use_progress_bar() { + Self::from_indicatif(indicatif::ProgressBar::new_spinner()) } else { - Self::hidden() + Self::no_progress_bar() } } - pub fn hidden() -> Self { - indicatif::ProgressBar::hidden().into() + fn from_indicatif(pb: indicatif::ProgressBar) -> Self { + let inner = Arc::new(pb); + logging::set_progress_bar(Some(Arc::downgrade(&inner))); + ProgressBar { + inner: Some(inner), + start: Instant::now(), + prefix: None, + } + } + + fn no_progress_bar() -> Self { + ProgressBar { + inner: None, + start: Instant::now(), + prefix: None, + } } pub fn finish_with_duration(&self, op: &str) { let dur = self.start.elapsed(); // We could use `dur.as_secs_f64()`, but its unnecessarily precise (micros). Millis are enough for our purpose. let msg = format!("{op} completed in {}s", dur.as_millis() as f64 / 1000.0); - let progress_style = ProgressStyle::default_bar().template("{prefix:.dim} {msg}"); - self.inner.set_style(progress_style); - self.inner.set_prefix(">"); - self.inner.finish_with_message(&msg); - logging::set_progress_bar(None); + + if let Some(inner) = &self.inner { + let progress_style = ProgressStyle::default_bar().template("{prefix:.dim} {msg}"); + inner.set_style(progress_style); + inner.set_prefix(">"); + inner.finish_with_message(&msg); + logging::set_progress_bar(None); + } else { + log::info!("> {msg}"); + } } pub fn finish_and_clear(&self) { - self.inner.finish_and_clear(); - logging::set_progress_bar(None); + if let Some(inner) = &self.inner { + inner.finish_and_clear(); + logging::set_progress_bar(None); + } } -} -impl From for ProgressBar { - fn from(pb: indicatif::ProgressBar) -> Self { - let inner = Arc::new(pb); - logging::set_progress_bar(Some(Arc::downgrade(&inner))); - ProgressBar { - inner, - start: Instant::now(), + pub fn set_style(&self, style: ProgressStyle) { + if let Some(inner) = &self.inner { + inner.set_style(style); + } + } + + pub fn set_message>(&self, msg: S) { + if let Some(inner) = &self.inner { + inner.set_message(msg.as_ref()); + } else { + log::debug!( + "{}{}", + self.prefix + .as_deref() + .map(|s| format!("{s} ")) + .unwrap_or_default(), + msg.as_ref() + ); } } -} -impl Deref for ProgressBar { - type Target = indicatif::ProgressBar; + pub fn tick(&self) { + if let Some(inner) = &self.inner { + inner.tick(); + } + } - fn deref(&self) -> &Self::Target { - &self.inner + pub fn set_prefix>(&mut self, prefix: S) { + if let Some(inner) = &self.inner { + inner.set_prefix(prefix.as_ref()); + } else { + self.prefix = Some(prefix.as_ref().to_owned()); + } + } + + pub fn set_position(&self, pos: u64) { + if let Some(inner) = &self.inner { + inner.set_position(pos); + } + } + + pub fn inc(&self, delta: u64) { + if let Some(inner) = &self.inner { + inner.inc(delta); + } + } + + pub fn enable_steady_tick(&self, interval: u64) { + if let Some(inner) = &self.inner { + inner.enable_steady_tick(interval); + } } } diff --git a/tests/integration/_cases/debug_files/upload/debug_files-upload-mixed-embedded-sources.trycmd b/tests/integration/_cases/debug_files/upload/debug_files-upload-mixed-embedded-sources.trycmd index 2bd43d4ac9..74bd2b1687 100644 --- a/tests/integration/_cases/debug_files/upload/debug_files-upload-mixed-embedded-sources.trycmd +++ b/tests/integration/_cases/debug_files/upload/debug_files-upload-mixed-embedded-sources.trycmd @@ -4,7 +4,10 @@ $ sentry-cli --log-level=debug debug-files upload --include-sources tests/integr INFO [..] Loaded config from [CWD]/.sentryclirc ... > Found 1 debug information file (1 with embedded sources) + DEBUG [..] > SrcGenSampleApp.pdb +... > Resolved source code for 0 debug information files + DEBUG [..] > SrcGenSampleApp.pdb > Prepared debug information file for upload ...