revert progress bar changes

Author: nathanwhit

cli/factory.rs

@@ -314,7 +314,7 @@ struct CliFactoryServices {
   resolver_factory: Deferred<Arc<CliResolverFactory>>,
   root_cert_store_provider: Deferred<Arc<dyn RootCertStoreProvider>>,
   root_permissions_container: Deferred<PermissionsContainer>,
-  text_only_progress_bar: Deferred<Option<ProgressBar>>,
+  text_only_progress_bar: Deferred<ProgressBar>,
   type_checker: Deferred<Arc<TypeChecker>>,
   workspace_factory: Deferred<Arc<CliWorkspaceFactory>>,
   install_reporter:
@@ -441,23 +441,11 @@ impl CliFactory {
     })
   }
 
-  pub fn text_only_progress_bar(
-    &self,
-  ) -> Result<Option<&ProgressBar>, AnyError> {
+  pub fn text_only_progress_bar(&self) -> &ProgressBar {
     self
       .services
       .text_only_progress_bar
-      .get_or_try_init(|| {
-        if matches!(
-          self.cli_options()?.sub_command(),
-          DenoSubcommand::Install(_)
-        ) {
-          Ok(None)
-        } else {
-          Ok(Some(ProgressBar::new(ProgressBarStyle::TextOnly)))
-        }
-      })
-      .map(|opt| opt.as_ref())
+      .get_or_init(|| ProgressBar::new(ProgressBarStyle::TextOnly))
   }
 
   pub fn global_http_cache(&self) -> Result<&Arc<GlobalHttpCache>, AnyError> {
@@ -504,15 +492,8 @@ impl CliFactory {
         CreateCliFileFetcherOptions {
           allow_remote: !cli_options.no_remote(),
           cache_setting: cli_options.cache_setting(),
-          download_log_level: if matches!(
-            cli_options.sub_command(),
-            DenoSubcommand::Install(_)
-          ) {
-            log::Level::Trace
-          } else {
-            log::Level::Info
-          },
-          progress_bar: self.text_only_progress_bar()?.cloned(),
+          download_log_level: log::Level::Info,
+          progress_bar: Some(self.text_only_progress_bar().clone()),
         },
       )))
     })
@@ -572,7 +553,7 @@ impl CliFactory {
         resolver_factory.clone(),
         Arc::new(CliNpmCacheHttpClient::new(
           self.http_client_provider().clone(),
-          self.text_only_progress_bar()?.cloned(),
+          self.text_only_progress_bar().clone(),
         )),
         match resolver_factory.npm_resolver()?.as_managed() {
           Some(managed_npm_resolver) => Arc::new(
@@ -581,7 +562,7 @@ impl CliFactory {
             as Arc<dyn LifecycleScriptsExecutor>,
           None => Arc::new(NullLifecycleScriptsExecutor),
         },
-        self.text_only_progress_bar()?.cloned(),
+        self.text_only_progress_bar().clone(),
         self
           .install_reporter()?
           .cloned()
@@ -608,7 +589,7 @@ impl CliFactory {
       .services
       .install_reporter
       .get_or_try_init(|| match self.cli_options()?.sub_command() {
-        DenoSubcommand::Install(_) => Ok(Some(Arc::new(
+        DenoSubcommand::Install(InstallFlags::Local(_)) => Ok(Some(Arc::new(
           crate::tools::installer::InstallReporter::new(),
         ))),
         _ => Ok(None),
@@ -787,7 +768,7 @@ impl CliFactory {
             self.npm_installer_if_managed().await?.cloned(),
             self.npm_resolver().await?.clone(),
             self.resolver_factory()?.parsed_source_cache().clone(),
-            self.text_only_progress_bar()?.cloned(),
+            self.text_only_progress_bar().clone(),
             self.resolver().await?.clone(),
             self.root_permissions_container()?.clone(),
             self.sys(),
@@ -866,7 +847,7 @@ impl CliFactory {
             cli_options.clone(),
             self.maybe_lockfile().await?.cloned(),
             self.module_graph_builder().await?.clone(),
-            self.text_only_progress_bar()?.cloned(),
+            self.text_only_progress_bar().clone(),
             self.type_checker().await?.clone(),
           )))
         }

cli/graph_util.rs

@@ -611,7 +611,7 @@ pub struct ModuleGraphBuilder {
   npm_installer: Option<Arc<CliNpmInstaller>>,
   npm_resolver: CliNpmResolver,
   parsed_source_cache: Arc<ParsedSourceCache>,
-  progress_bar: Option<ProgressBar>,
+  progress_bar: ProgressBar,
   resolver: Arc<CliResolver>,
   root_permissions_container: PermissionsContainer,
   sys: CliSys,
@@ -635,7 +635,7 @@ impl ModuleGraphBuilder {
     npm_installer: Option<Arc<CliNpmInstaller>>,
     npm_resolver: CliNpmResolver,
     parsed_source_cache: Arc<ParsedSourceCache>,
-    progress_bar: Option<ProgressBar>,
+    progress_bar: ProgressBar,
     resolver: Arc<CliResolver>,
     root_permissions_container: PermissionsContainer,
     sys: CliSys,
@@ -684,10 +684,7 @@ impl ModuleGraphBuilder {
       }
     }
 
-    let _clear_guard = self
-      .progress_bar
-      .as_ref()
-      .map(|p| p.deferred_keep_initialize_alive());
+    let _clear_guard = self.progress_bar.deferred_keep_initialize_alive();
     let analyzer = self.module_info_cache.as_module_analyzer();
     let mut loader = match options.loader {
       Some(loader) => MutLoaderRef::Borrowed(loader),

cli/http_util.rs

@@ -269,10 +269,10 @@ impl HttpClient {
     &self,
     url: Url,
     headers: &HeaderMap,
-    progress_guard: Option<&UpdateGuard>,
+    progress_guard: &UpdateGuard,
   ) -> Result<HttpClientResponse, DownloadError> {
     crate::util::retry::retry(
-      || self.download_inner(url.clone(), headers, progress_guard),
+      || self.download_inner(url.clone(), headers, Some(progress_guard)),
       |e| {
         matches!(
           e.as_kind(),

cli/lsp/config.rs

@@ -1456,10 +1456,10 @@ impl ConfigData {
           .cloned()
           // will only happen in the tests
           .unwrap_or_else(|| Arc::new(HttpClientProvider::new(None, None))),
-        Some(pb.clone()),
+        pb.clone(),
       )),
       Arc::new(NullLifecycleScriptsExecutor),
-      Some(pb),
+      pb,
       None,
       NpmInstallerFactoryOptions {
         cache_setting: NpmCacheSetting::Use,

cli/lsp/resolver.rs

@@ -861,7 +861,7 @@ impl<'a> ResolverFactory<'a> {
       let pb = ProgressBar::new(ProgressBarStyle::TextOnly);
       let npm_client = Arc::new(CliNpmCacheHttpClient::new(
         http_client_provider.clone(),
-        Some(pb.clone()),
+        pb.clone(),
       ));
       let registry_info_provider = Arc::new(CliNpmRegistryInfoProvider::new(
         npm_cache.clone(),
@@ -917,7 +917,7 @@ impl<'a> ResolverFactory<'a> {
         self.services.npm_resolution.clone(),
         npm_resolution_initializer.clone(),
         npm_resolution_installer,
-        Some(&pb),
+        &pb,
         sys.clone(),
         tarball_cache.clone(),
         maybe_lockfile,

cli/module_loader.rs

@@ -130,7 +130,7 @@ pub struct ModuleLoadPreparer {
   options: Arc<CliOptions>,
   lockfile: Option<Arc<CliLockfile>>,
   module_graph_builder: Arc<ModuleGraphBuilder>,
-  progress_bar: Option<ProgressBar>,
+  progress_bar: ProgressBar,
   type_checker: Arc<TypeChecker>,
 }
 
@@ -152,7 +152,7 @@ impl ModuleLoadPreparer {
     options: Arc<CliOptions>,
     lockfile: Option<Arc<CliLockfile>>,
     module_graph_builder: Arc<ModuleGraphBuilder>,
-    progress_bar: Option<ProgressBar>,
+    progress_bar: ProgressBar,
     type_checker: Arc<TypeChecker>,
   ) -> Self {
     Self {
@@ -183,10 +183,7 @@ impl ModuleLoadPreparer {
       allow_unknown_media_types,
       skip_graph_roots_validation,
     } = options;
-    let _pb_clear_guard = self
-      .progress_bar
-      .as_ref()
-      .map(|pb| pb.deferred_keep_initialize_alive());
+    let _pb_clear_guard = self.progress_bar.deferred_keep_initialize_alive();
 
     let mut loader = self
       .module_graph_builder
@@ -276,10 +273,7 @@ impl ModuleLoadPreparer {
         .collect::<Vec<_>>()
         .join(", ")
     );
-    let _pb_clear_guard = self
-      .progress_bar
-      .as_ref()
-      .map(|pb| pb.deferred_keep_initialize_alive());
+    let _pb_clear_guard = self.progress_bar.deferred_keep_initialize_alive();
 
     let mut loader = self
       .module_graph_builder

cli/npm.rs

@@ -66,13 +66,13 @@ pub type CliNpmGraphResolver = deno_npm_installer::graph::NpmDenoGraphResolver<
 #[derive(Debug)]
 pub struct CliNpmCacheHttpClient {
   http_client_provider: Arc<HttpClientProvider>,
-  progress_bar: Option<ProgressBar>,
+  progress_bar: ProgressBar,
 }
 
 impl CliNpmCacheHttpClient {
   pub fn new(
     http_client_provider: Arc<HttpClientProvider>,
-    progress_bar: Option<ProgressBar>,
+    progress_bar: ProgressBar,
   ) -> Self {
     Self {
       http_client_provider,
@@ -89,7 +89,7 @@ impl deno_npm_cache::NpmCacheHttpClient for CliNpmCacheHttpClient {
     maybe_auth: Option<String>,
     maybe_etag: Option<String>,
   ) -> Result<NpmCacheHttpClientResponse, deno_npm_cache::DownloadError> {
-    let guard = self.progress_bar.as_ref().map(|pb| pb.update(url.as_str()));
+    let guard = self.progress_bar.update(url.as_str());
     let client = self.http_client_provider.get_or_create().map_err(|err| {
       deno_npm_cache::DownloadError {
         status_code: None,
@@ -110,7 +110,7 @@ impl deno_npm_cache::NpmCacheHttpClient for CliNpmCacheHttpClient {
       );
     }
     client
-      .download_with_progress_and_retries(url, &headers, guard.as_ref())
+      .download_with_progress_and_retries(url, &headers, &guard)
       .await
       .map(|response| match response {
         crate::http_util::HttpClientResponse::Success { headers, body } => {

cli/standalone/binary.rs

@@ -339,7 +339,7 @@ impl<'a> DenoCompileBinaryWriter<'a> {
         .download_with_progress_and_retries(
           download_url.parse()?,
           &Default::default(),
-          Some(&progress),
+          &progress,
         )
         .await?
     };

cli/tools/installer/mod.rs

@@ -11,7 +11,6 @@ use std::os::unix::fs::PermissionsExt;
 use std::path::Path;
 use std::path::PathBuf;
 use std::sync::Arc;
-use std::sync::atomic::AtomicBool;
 use std::sync::atomic::AtomicUsize;
 use std::sync::atomic::Ordering;
 
@@ -21,7 +20,6 @@ use deno_core::anyhow::Context;
 use deno_core::anyhow::anyhow;
 use deno_core::anyhow::bail;
 use deno_core::error::AnyError;
-use deno_core::parking_lot::Mutex;
 use deno_core::url::Url;
 use deno_lib::args::CaData;
 use deno_path_util::resolve_url_or_path;
@@ -415,46 +413,6 @@ pub(crate) async fn install_from_entrypoints(
     .await
 }
 
-#[derive(Debug)]
-struct InstallProgressBar {
-  stats: Arc<InstallStats>,
-  done: Mutex<tokio::sync::oneshot::Receiver<()>>,
-  done_for_sure: AtomicBool,
-}
-
-impl crate::util::draw_thread::DrawThreadRenderer for InstallProgressBar {
-  fn render(&self, _size: &deno_runtime::ops::tty::ConsoleSize) -> String {
-    let done = self.done.lock().try_recv().is_ok()
-      || self.done_for_sure.load(Ordering::Relaxed);
-    if done {
-      self.done_for_sure.store(true, Ordering::Relaxed);
-      return "".to_string();
-    }
-    format!(
-      "Progress: resolved {}, reused: {}, downloaded: {}, added: {}{}",
-      deno_terminal::colors::cyan(
-        self.stats.resolved_npm.len() + self.stats.resolved_jsr.len()
-      ),
-      deno_terminal::colors::cyan(
-        self.stats.reused_npm.get() + self.stats.reused_jsr.len()
-      ),
-      deno_terminal::colors::cyan(
-        self.stats.downloaded_npm.get() + self.stats.downloaded_jsr.len()
-      ),
-      deno_terminal::colors::cyan(
-        self.stats.intialized_npm.len() + self.stats.downloaded_jsr.len()
-      ),
-      if self.done.lock().try_recv().is_ok()
-        || self.done_for_sure.load(Ordering::Relaxed)
-      {
-        ", done"
-      } else {
-        ""
-      }
-    )
-  }
-}
-
 async fn install_local(
   flags: Arc<Flags>,
   install_flags: InstallFlagsLocal,
@@ -484,16 +442,6 @@ async fn install_top_level(factory: &CliFactory) -> Result<(), AnyError> {
 
   // set up the custom progress bar
   let install_reporter = factory.install_reporter()?.unwrap().clone();
-  let (done_tx, done_rx) = tokio::sync::oneshot::channel();
-  let progress_bar = Arc::new(InstallProgressBar {
-    stats: install_reporter.stats.clone(),
-    done: Mutex::new(done_rx),
-    done_for_sure: AtomicBool::new(false),
-  });
-
-  let _guard =
-    crate::util::draw_thread::DrawThread::add_entry(progress_bar.clone());
-  crate::util::draw_thread::DrawThread::show();
 
   // the actual work
   crate::tools::pm::cache_top_level_deps(factory, None).await?;
@@ -557,9 +505,6 @@ async fn install_top_level(factory: &CliFactory) -> Result<(), AnyError> {
     }
   }
 
-  done_tx.send(()).unwrap();
-  crate::util::draw_thread::DrawThread::hide();
-
   let mut installed_normal_deps = Vec::new();
   let mut installed_dev_deps = Vec::new();
 

cli/tools/pm/cache_deps.rs

@@ -26,9 +26,8 @@ pub async fn cache_top_level_deps(
   jsr_resolver: Option<Arc<crate::jsr::JsrFetchResolver>>,
 ) -> Result<(), AnyError> {
   let _clear_guard = factory
-    .text_only_progress_bar()?
-    .as_ref()
-    .map(|pb| pb.deferred_keep_initialize_alive());
+    .text_only_progress_bar()
+    .deferred_keep_initialize_alive();
   let npm_installer = factory.npm_installer().await?;
   npm_installer
     .ensure_top_level_package_json_install()