diff --git a/src/cli.rs b/src/cli.rs index 96f073e7f6..88a7586ce2 100644 --- a/src/cli.rs +++ b/src/cli.rs @@ -2,7 +2,6 @@ #[macro_use] pub mod log; pub mod common; -mod download_tracker; pub mod errors; mod help; mod job; diff --git a/src/cli/common.rs b/src/cli/common.rs index bb5002cf9c..46e722ad29 100644 --- a/src/cli/common.rs +++ b/src/cli/common.rs @@ -1,29 +1,26 @@ //! Just a dumping ground for cli stuff -use std::cell::RefCell; use std::fmt::Display; use std::fs; use std::io::{BufRead, Write}; use std::path::{Path, PathBuf}; -use std::sync::{Arc, LazyLock, Mutex}; +use std::sync::LazyLock; use std::{cmp, env}; use anyhow::{Context, Result, anyhow}; use git_testament::{git_testament, render_testament}; use termcolor::Color; -use tracing::{debug, error, info, trace, warn}; +use tracing::{error, info, warn}; use tracing_subscriber::{EnvFilter, Registry, reload::Handle}; use crate::{ - cli::download_tracker::DownloadTracker, config::Cfg, dist::{TargetTriple, ToolchainDesc}, errors::RustupError, install::UpdateStatus, - notifications::Notification, process::{Attr, Process}, toolchain::{LocalToolchainName, Toolchain, ToolchainName}, - utils::{self, notify::NotificationLevel}, + utils, }; pub(crate) const WARN_COMPLETE_PROFILE: &str = "downloading with complete profile isn't recommended unless you are a developer of the rust language"; @@ -122,58 +119,9 @@ pub(crate) fn read_line(process: &Process) -> Result { .context("unable to read from stdin for confirmation") } -pub(super) struct Notifier { - tracker: Mutex, - ram_notice_shown: RefCell, -} - -impl Notifier { - pub(super) fn new(quiet: bool, process: &Process) -> Self { - Self { - tracker: Mutex::new(DownloadTracker::new_with_display_progress(!quiet, process)), - ram_notice_shown: RefCell::new(false), - } - } - - pub(super) fn handle(&self, n: Notification<'_>) { - if self.tracker.lock().unwrap().handle_notification(&n) { - return; - } - - if let Notification::SetDefaultBufferSize(_) = &n { - if *self.ram_notice_shown.borrow() { - return; - } else { - *self.ram_notice_shown.borrow_mut() = true; - } - }; - let level = n.level(); - for n in format!("{n}").lines() { - match level { - NotificationLevel::Debug => { - debug!("{}", n); - } - NotificationLevel::Info => { - info!("{}", n); - } - NotificationLevel::Warn => { - warn!("{}", n); - } - NotificationLevel::Error => { - error!("{}", n); - } - NotificationLevel::Trace => { - trace!("{}", n); - } - } - } - } -} - #[tracing::instrument(level = "trace", skip(process))] pub(crate) fn set_globals(current_dir: PathBuf, quiet: bool, process: &Process) -> Result> { - let notifier = Notifier::new(quiet, process); - Cfg::from_env(current_dir, Arc::new(move |n| notifier.handle(n)), process) + Cfg::from_env(current_dir, quiet, process) } pub(crate) fn show_channel_update( diff --git a/src/cli/download_tracker.rs b/src/cli/download_tracker.rs deleted file mode 100644 index a2105dd048..0000000000 --- a/src/cli/download_tracker.rs +++ /dev/null @@ -1,149 +0,0 @@ -use indicatif::{MultiProgress, ProgressBar, ProgressDrawTarget, ProgressStyle}; -use std::collections::HashMap; -use std::time::{Duration, Instant}; - -use crate::notifications::Notification; -use crate::process::Process; - -/// Tracks download progress and displays information about it to a terminal. -/// -/// *not* safe for tracking concurrent downloads yet - it is basically undefined -/// what will happen. -pub(crate) struct DownloadTracker { - /// MultiProgress bar for the downloads. - multi_progress_bars: MultiProgress, - /// Mapping of URLs being downloaded to their corresponding progress bars. - /// The `Option` represents the instant where the download is being retried, - /// allowing us delay the reappearance of the progress bar so that the user can see - /// the message "retrying download" for at least a second. - /// Without it, the progress bar would reappear immediately, not allowing the user to - /// correctly see the message, before the progress bar starts again. - file_progress_bars: HashMap)>, -} - -impl DownloadTracker { - /// Creates a new DownloadTracker. - pub(crate) fn new_with_display_progress(display_progress: bool, process: &Process) -> Self { - let multi_progress_bars = MultiProgress::with_draw_target(if display_progress { - process.progress_draw_target() - } else { - ProgressDrawTarget::hidden() - }); - - Self { - multi_progress_bars, - file_progress_bars: HashMap::new(), - } - } - - pub(crate) fn handle_notification(&mut self, n: &Notification<'_>) -> bool { - match *n { - Notification::DownloadContentLengthReceived(content_len, url) => { - if let Some(url) = url { - self.content_length_received(content_len, url); - } - true - } - Notification::DownloadDataReceived(data, url) => { - if let Some(url) = url { - self.data_received(data.len(), url); - } - true - } - Notification::DownloadFinished(url) => { - if let Some(url) = url { - self.download_finished(url); - } - true - } - Notification::DownloadFailed(url) => { - self.download_failed(url); - false - } - Notification::DownloadingComponent(component, _, _, url) => { - self.create_progress_bar(component.to_owned(), url.to_owned()); - true - } - Notification::RetryingDownload(url) => { - self.retrying_download(url); - true - } - _ => false, - } - } - - /// Creates a new ProgressBar for the given component. - pub(crate) fn create_progress_bar(&mut self, component: String, url: String) { - let pb = ProgressBar::hidden(); - pb.set_style( - ProgressStyle::with_template( - "{msg:>12.bold} [{bar:40}] {bytes}/{total_bytes} ({bytes_per_sec}, ETA: {eta})", - ) - .unwrap() - .progress_chars("## "), - ); - pb.set_message(component); - self.multi_progress_bars.add(pb.clone()); - self.file_progress_bars.insert(url, (pb, None)); - } - - /// Sets the length for a new ProgressBar and gives it a style. - pub(crate) fn content_length_received(&mut self, content_len: u64, url: &str) { - if let Some((pb, _)) = self.file_progress_bars.get(url) { - pb.reset(); - pb.set_length(content_len); - } - } - - /// Notifies self that data of size `len` has been received. - pub(crate) fn data_received(&mut self, len: usize, url: &str) { - let Some((pb, retry_time)) = self.file_progress_bars.get_mut(url) else { - return; - }; - pb.inc(len as u64); - if !retry_time.is_some_and(|instant| instant.elapsed() > Duration::from_secs(1)) { - return; - } - *retry_time = None; - pb.set_style( - ProgressStyle::with_template( - "{msg:>12.bold} [{bar:40}] {bytes}/{total_bytes} ({bytes_per_sec}, ETA: {eta})", - ) - .unwrap() - .progress_chars("## "), - ); - } - - /// Notifies self that the download has finished. - pub(crate) fn download_finished(&mut self, url: &str) { - let Some((pb, _)) = self.file_progress_bars.get(url) else { - return; - }; - pb.set_style( - ProgressStyle::with_template("{msg:>12.bold} downloaded {total_bytes} in {elapsed}") - .unwrap(), - ); - pb.finish(); - } - - /// Notifies self that the download has failed. - pub(crate) fn download_failed(&mut self, url: &str) { - let Some((pb, _)) = self.file_progress_bars.get(url) else { - return; - }; - pb.set_style( - ProgressStyle::with_template("{msg:>12.bold} download failed after {elapsed}") - .unwrap(), - ); - pb.finish(); - } - - /// Notifies self that the download is being retried. - pub(crate) fn retrying_download(&mut self, url: &str) { - let Some((pb, retry_time)) = self.file_progress_bars.get_mut(url) else { - return; - }; - *retry_time = Some(Instant::now()); - pb.set_style(ProgressStyle::with_template("{msg:>12.bold} retrying download").unwrap()); - } -} diff --git a/src/cli/rustup_mode.rs b/src/cli/rustup_mode.rs index 46df7df561..1c7b7beaac 100644 --- a/src/cli/rustup_mode.rs +++ b/src/cli/rustup_mode.rs @@ -34,6 +34,7 @@ use crate::{ config::{ActiveReason, Cfg}, dist::{ AutoInstallMode, PartialToolchainDesc, Profile, TargetTriple, + download::DownloadCfg, manifest::{Component, ComponentStatus}, }, errors::RustupError, @@ -909,7 +910,7 @@ async fn check_updates(cfg: &Cfg<'_>, opts: CheckOpts) -> Result Result> { /// `CARGO_HOME`/bin, hard-linking the various Rust tools to it, /// and adding `CARGO_HOME`/bin to PATH. pub(crate) async fn install( - current_dir: PathBuf, no_prompt: bool, - quiet: bool, mut opts: InstallOpts<'_>, - process: &Process, + cfg: &mut Cfg<'_>, ) -> Result { #[cfg_attr(not(unix), allow(unused_mut))] let mut exit_code = utils::ExitCode(0); - opts.validate(process).map_err(|e| { + opts.validate(cfg.process).map_err(|e| { anyhow!( "Pre-checks for host and toolchain failed: {e}\n\ If you are unsure of suitable values, the 'stable' toolchain is the default.\n\ Valid host triples look something like: {}", - TargetTriple::from_host_or_build(process) + TargetTriple::from_host_or_build(cfg.process) ) })?; - if process + if cfg + .process .var_os("RUSTUP_INIT_SKIP_EXISTENCE_CHECKS") .is_none_or(|s| s != "yes") { - check_existence_of_rustc_or_cargo_in_path(no_prompt, process)?; - check_existence_of_settings_file(process)?; + check_existence_of_rustc_or_cargo_in_path(no_prompt, cfg.process)?; + check_existence_of_settings_file(cfg.process)?; } #[cfg(unix)] { - exit_code &= unix::do_anti_sudo_check(no_prompt, process)?; + exit_code &= unix::do_anti_sudo_check(no_prompt, cfg.process)?; } - let mut term = process.stdout(); + let mut term = cfg.process.stdout(); #[cfg(windows)] - windows::maybe_install_msvc(&mut term, no_prompt, quiet, &opts, process).await?; + windows::maybe_install_msvc(&mut term, no_prompt, &opts, &*cfg).await?; if !no_prompt { - let msg = pre_install_msg(opts.no_modify_path, process)?; + let msg = pre_install_msg(opts.no_modify_path, cfg.process)?; md(&mut term, msg); let mut customized_install = false; loop { - md(&mut term, current_install_opts(&opts, process)); - match common::confirm_advanced(customized_install, process)? { + md(&mut term, current_install_opts(&opts, cfg.process)); + match common::confirm_advanced(customized_install, cfg.process)? { Confirm::No => { info!("aborting installation"); return Ok(utils::ExitCode(0)); @@ -561,15 +561,15 @@ pub(crate) async fn install( } Confirm::Advanced => { customized_install = true; - opts.customize(process)?; + opts.customize(cfg.process)?; } } } } let no_modify_path = opts.no_modify_path; - if let Err(e) = maybe_install_rust(current_dir, quiet, opts, process).await { - report_error(&e, process); + if let Err(e) = maybe_install_rust(opts, cfg).await { + report_error(&e, cfg.process); // On windows, where installation happens in a console // that may have opened just for this purpose, give @@ -577,13 +577,13 @@ pub(crate) async fn install( // window closes. #[cfg(windows)] if !no_prompt { - windows::ensure_prompt(process)?; + windows::ensure_prompt(cfg.process)?; } return Ok(utils::ExitCode(1)); } - let cargo_home = canonical_cargo_home(process)?; + let cargo_home = canonical_cargo_home(cfg.process)?; #[cfg(windows)] let cargo_home = cargo_home.replace('\\', r"\\"); #[cfg(windows)] @@ -596,7 +596,7 @@ pub(crate) async fn install( format!(post_install_msg_win!(), cargo_home = cargo_home) }; #[cfg(not(windows))] - let cargo_home_nushell = Nu.cargo_home_str(process)?; + let cargo_home_nushell = Nu.cargo_home_str(cfg.process)?; #[cfg(not(windows))] let msg = if no_modify_path { format!( @@ -614,14 +614,14 @@ pub(crate) async fn install( md(&mut term, msg); #[cfg(unix)] - warn_if_default_linker_missing(process); + warn_if_default_linker_missing(cfg.process); #[cfg(windows)] if !no_prompt { // On windows, where installation happens in a console // that may have opened just for this purpose, require // the user to press a key to continue. - windows::ensure_prompt(process)?; + windows::ensure_prompt(cfg.process)?; } Ok(exit_code) @@ -911,24 +911,20 @@ fn check_proxy_sanity(process: &Process, components: &[&str], desc: &ToolchainDe Ok(()) } -async fn maybe_install_rust( - current_dir: PathBuf, - quiet: bool, - opts: InstallOpts<'_>, - process: &Process, -) -> Result<()> { - install_bins(process)?; +async fn maybe_install_rust(opts: InstallOpts<'_>, cfg: &mut Cfg<'_>) -> Result<()> { + install_bins(cfg.process)?; #[cfg(unix)] - unix::do_write_env_files(process)?; + unix::do_write_env_files(cfg.process)?; if !opts.no_modify_path { - do_add_to_path(process)?; + do_add_to_path(cfg.process)?; } // If RUSTUP_HOME is not set, make sure it exists - if process.var_os("RUSTUP_HOME").is_none() { - let home = process + if cfg.process.var_os("RUSTUP_HOME").is_none() { + let home = cfg + .process .home_dir() .map(|p| p.join(".rustup")) .ok_or_else(|| anyhow::anyhow!("could not find home dir to put .rustup in"))?; @@ -936,25 +932,23 @@ async fn maybe_install_rust( fs::create_dir_all(home).context("unable to create ~/.rustup")?; } - let mut cfg = common::set_globals(current_dir, quiet, process)?; - let (components, targets) = (opts.components, opts.targets); - let toolchain = opts.install(&mut cfg)?; + let toolchain = opts.install(cfg)?; if let Some(desc) = &toolchain { - let status = if Toolchain::exists(&cfg, &desc.into())? { + let status = if Toolchain::exists(cfg, &desc.into())? { warn!("Updating existing toolchain, profile choice will be ignored"); // If we have a partial install we might not be able to read content here. We could: // - fail and folk have to delete the partially present toolchain to recover // - silently ignore it (and provide inconsistent metadata for reporting the install/update change) // - delete the partial install and start over // For now, we error. - let mut toolchain = DistributableToolchain::new(&cfg, desc.clone())?; + let mut toolchain = DistributableToolchain::new(cfg, desc.clone())?; toolchain .update(components, targets, cfg.get_profile()?) .await? } else { DistributableToolchain::install( - &cfg, + cfg, desc, components, targets, @@ -965,11 +959,11 @@ async fn maybe_install_rust( .0 }; - check_proxy_sanity(process, components, desc)?; + check_proxy_sanity(cfg.process, components, desc)?; cfg.set_default(Some(&desc.into()))?; - writeln!(process.stdout().lock())?; - common::show_channel_update(&cfg, PackageUpdate::Toolchain(desc.clone()), Ok(status))?; + writeln!(cfg.process.stdout().lock())?; + common::show_channel_update(cfg, PackageUpdate::Toolchain(desc.clone()), Ok(status))?; } Ok(()) } @@ -1112,7 +1106,7 @@ pub(crate) fn self_update_permitted(explicit: bool) -> Result Result { +pub(crate) async fn self_update(dl_cfg: &DownloadCfg<'_>) -> Result { match self_update_permitted(false)? { SelfUpdatePermission::HardFail => { error!("Unable to self-update. STOP"); @@ -1123,13 +1117,13 @@ pub(crate) async fn self_update(process: &Process) -> Result { SelfUpdatePermission::Permit => {} } - let setup_path = prepare_update(process).await?; + let setup_path = prepare_update(dl_cfg).await?; if let Some(setup_path) = &setup_path { return run_update(setup_path); } else { // Try again in case we emitted "tool `{}` is already installed" last time. - install_proxies(process)?; + install_proxies(dl_cfg.process)?; } Ok(utils::ExitCode(0)) @@ -1174,7 +1168,7 @@ pub(crate) async fn update(cfg: &Cfg<'_>) -> Result { Permit => {} } - match prepare_update(cfg.process).await? { + match prepare_update(&DownloadCfg::new(cfg)).await? { Some(setup_path) => { let Some(version) = get_and_parse_new_rustup_version(&setup_path) else { error!("failed to get rustup version"); @@ -1227,8 +1221,8 @@ fn parse_new_rustup_version(version: String) -> String { String::from(matched_version) } -pub(crate) async fn prepare_update(process: &Process) -> Result> { - let cargo_home = process.cargo_home()?; +pub(crate) async fn prepare_update(dl_cfg: &DownloadCfg<'_>) -> Result> { + let cargo_home = dl_cfg.process.cargo_home()?; let rustup_path = cargo_home.join(format!("bin{MAIN_SEPARATOR}rustup{EXE_SUFFIX}")); let setup_path = cargo_home.join(format!("bin{MAIN_SEPARATOR}rustup-init{EXE_SUFFIX}")); @@ -1250,22 +1244,22 @@ pub(crate) async fn prepare_update(process: &Process) -> Result> // If someone really wants to use another version, they still can enforce // that using the environment variable RUSTUP_OVERRIDE_HOST_TRIPLE. #[cfg(windows)] - let triple = dist::TargetTriple::from_host(process).unwrap_or(triple); + let triple = dist::TargetTriple::from_host(dl_cfg.process).unwrap_or(triple); // Get update root. - let update_root = update_root(process); + let update_root = update_root(dl_cfg.process); // Get current version let current_version = env!("CARGO_PKG_VERSION"); // Get available version info!("checking for self-update (current version: {current_version})"); - let available_version = match process.var_opt("RUSTUP_VERSION")? { + let available_version = match dl_cfg.process.var_opt("RUSTUP_VERSION")? { Some(ver) => { info!("`RUSTUP_VERSION` has been set to `{ver}`"); ver } - None => get_available_rustup_version(process).await?, + None => get_available_rustup_version(dl_cfg).await?, }; // If up-to-date @@ -1281,7 +1275,14 @@ pub(crate) async fn prepare_update(process: &Process) -> Result> // Download new version info!("downloading self-update (new version: {available_version})"); - download_file(&download_url, &setup_path, None, &|_| (), process).await?; + download_file( + &download_url, + &setup_path, + None, + &dl_cfg.notifier, + dl_cfg.process, + ) + .await?; // Mark as executable utils::make_executable(&setup_path)?; @@ -1289,8 +1290,8 @@ pub(crate) async fn prepare_update(process: &Process) -> Result> Ok(Some(setup_path)) } -async fn get_available_rustup_version(process: &Process) -> Result { - let update_root = update_root(process); +async fn get_available_rustup_version(dl_cfg: &DownloadCfg<'_>) -> Result { + let update_root = update_root(dl_cfg.process); let tempdir = tempfile::Builder::new() .prefix("rustup-update") .tempdir() @@ -1300,7 +1301,14 @@ async fn get_available_rustup_version(process: &Process) -> Result { let release_file_url = format!("{update_root}/release-stable.toml"); let release_file_url = utils::parse_url(&release_file_url)?; let release_file = tempdir.path().join("release-stable.toml"); - download_file(&release_file_url, &release_file, None, &|_| (), process).await?; + download_file( + &release_file_url, + &release_file, + None, + &dl_cfg.notifier, + dl_cfg.process, + ) + .await?; let release_toml_str = utils::read_file("rustup release", &release_file)?; let release_toml = toml::from_str::(&release_toml_str) .context("unable to parse rustup release file")?; @@ -1348,13 +1356,13 @@ impl fmt::Display for SchemaVersion { } /// Returns whether an update was available -pub(crate) async fn check_rustup_update(process: &Process) -> anyhow::Result { - let mut t = process.stdout(); +pub(crate) async fn check_rustup_update(dl_cfg: &DownloadCfg<'_>) -> anyhow::Result { + let mut t = dl_cfg.process.stdout(); // Get current rustup version let current_version = env!("CARGO_PKG_VERSION"); // Get available rustup version - let available_version = get_available_rustup_version(process).await?; + let available_version = get_available_rustup_version(dl_cfg).await?; let _ = t.attr(Attr::Bold); write!(t.lock(), "rustup - ")?; diff --git a/src/cli/self_update/windows.rs b/src/cli/self_update/windows.rs index 1b4c48a81d..499bd1225e 100644 --- a/src/cli/self_update/windows.rs +++ b/src/cli/self_update/windows.rs @@ -5,9 +5,8 @@ use std::io::Write; use std::os::windows::ffi::OsStrExt; use std::path::Path; use std::process::Command; -use std::sync::{Arc, Mutex}; #[cfg(any(test, feature = "test"))] -use std::sync::{LockResult, MutexGuard}; +use std::sync::{LockResult, Mutex, MutexGuard}; use anyhow::{Context, Result, anyhow}; use tracing::{info, warn}; @@ -20,8 +19,10 @@ use windows_sys::Win32::Foundation::{ERROR_FILE_NOT_FOUND, ERROR_INVALID_DATA}; use super::super::errors::*; use super::common; use super::{InstallOpts, install_bins, report_error}; -use crate::cli::{download_tracker::DownloadTracker, markdown::md}; +use crate::cli::markdown::md; +use crate::config::Cfg; use crate::dist::TargetTriple; +use crate::dist::download::DownloadCfg; use crate::download::download_file; use crate::process::{ColorableTerminal, Process}; use crate::utils; @@ -91,36 +92,35 @@ pub(crate) fn choose_vs_install(process: &Process) -> Result, - process: &Process, + cfg: &Cfg<'_>, ) -> Result<()> { - let Some(plan) = do_msvc_check(opts, process) else { + let Some(plan) = do_msvc_check(opts, cfg.process) else { return Ok(()); }; if no_prompt { warn!("installing msvc toolchain without its prerequisites"); - } else if !quiet && plan == VsInstallPlan::Automatic { + } else if !cfg.quiet && plan == VsInstallPlan::Automatic { md(term, MSVC_AUTO_INSTALL_MESSAGE); - match choose_vs_install(process)? { + match choose_vs_install(cfg.process)? { Some(VsInstallPlan::Automatic) => { - match try_install_msvc(opts, process).await { + match try_install_msvc(opts, cfg).await { Err(e) => { // Make sure the console doesn't exit before the user can // see the error and give the option to continue anyway. - report_error(&e, process); - if !common::question_bool("\nContinue?", false, process)? { + report_error(&e, cfg.process); + if !common::question_bool("\nContinue?", false, cfg.process)? { info!("aborting installation"); } } - Ok(ContinueInstall::No) => ensure_prompt(process)?, + Ok(ContinueInstall::No) => ensure_prompt(cfg.process)?, _ => {} } } Some(VsInstallPlan::Manual) => { md(term, MSVC_MANUAL_INSTALL_MESSAGE); - if !common::question_bool("\nContinue?", false, process)? { + if !common::question_bool("\nContinue?", false, cfg.process)? { info!("aborting installation"); } } @@ -129,7 +129,7 @@ pub(super) async fn maybe_install_msvc( } else { md(term, MSVC_MESSAGE); md(term, MSVC_MANUAL_INSTALL_MESSAGE); - if !common::question_bool("\nContinue?", false, process)? { + if !common::question_bool("\nContinue?", false, cfg.process)? { info!("aborting installation"); } } @@ -260,7 +260,7 @@ pub(crate) enum ContinueInstall { /// but the rustup install should not be continued at this time. pub(crate) async fn try_install_msvc( opts: &InstallOpts<'_>, - process: &Process, + cfg: &Cfg<'_>, ) -> Result { // download the installer let visual_studio_url = utils::parse_url("https://aka.ms/vs/17/release/vs_community.exe")?; @@ -271,23 +271,14 @@ pub(crate) async fn try_install_msvc( .context("error creating temp directory")?; let visual_studio = tempdir.path().join("vs_setup.exe"); - let download_tracker = Arc::new(Mutex::new(DownloadTracker::new_with_display_progress( - true, process, - ))); - download_tracker - .lock() - .unwrap() - .download_finished(visual_studio_url.as_str()); - + let dl_cfg = DownloadCfg::new(cfg); info!("downloading Visual Studio installer"); download_file( &visual_studio_url, &visual_studio, None, - &move |n| { - download_tracker.lock().unwrap().handle_notification(&n); - }, - process, + &dl_cfg.notifier, + dl_cfg.process, ) .await?; @@ -304,7 +295,7 @@ pub(crate) async fn try_install_msvc( // It's possible an earlier or later version of the Windows SDK has been // installed separately from Visual Studio so installing it can be skipped. - if !has_windows_sdk_libs(process) { + if !has_windows_sdk_libs(cfg.process) { cmd.args([ "--add", "Microsoft.VisualStudio.Component.Windows11SDK.22000", @@ -337,8 +328,8 @@ pub(crate) async fn try_install_msvc( // It's possible that the installer returned a non-zero exit code // even though the required components were successfully installed. // In that case we warn about the error but continue on. - let have_msvc = do_msvc_check(opts, process).is_none(); - let has_libs = has_windows_sdk_libs(process); + let have_msvc = do_msvc_check(opts, cfg.process).is_none(); + let has_libs = has_windows_sdk_libs(cfg.process); if have_msvc && has_libs { warn!("Visual Studio is installed but a problem occurred during installation"); warn!("{}", err); diff --git a/src/cli/setup_mode.rs b/src/cli/setup_mode.rs index 0cabf4aeef..38bf829231 100644 --- a/src/cli/setup_mode.rs +++ b/src/cli/setup_mode.rs @@ -128,5 +128,6 @@ pub async fn main( targets: &target.iter().map(|s| &**s).collect::>(), }; - self_update::install(current_dir, no_prompt, quiet, opts, process).await + let mut cfg = common::set_globals(current_dir, quiet, process)?; + self_update::install(no_prompt, opts, &mut cfg).await } diff --git a/src/config.rs b/src/config.rs index 9f238038d0..4f985b9f32 100644 --- a/src/config.rs +++ b/src/config.rs @@ -2,7 +2,6 @@ use std::fmt::{self, Debug, Display}; use std::io; use std::path::{Path, PathBuf}; use std::str::FromStr; -use std::sync::Arc; use anyhow::{Context, Result, anyhow, bail}; use serde::Deserialize; @@ -13,14 +12,10 @@ use tracing::{debug, error, info, trace, warn}; use crate::dist::AutoInstallMode; use crate::{ cli::{common, self_update::SelfUpdateMode}, - dist::{ - self, PartialToolchainDesc, Profile, TargetTriple, ToolchainDesc, download::DownloadCfg, - temp, - }, + dist::{self, PartialToolchainDesc, Profile, TargetTriple, ToolchainDesc, temp}, errors::RustupError, fallback_settings::FallbackSettings, install::UpdateStatus, - notifications::*, process::Process, settings::{MetadataVersion, Settings, SettingsFile}, toolchain::{ @@ -233,7 +228,7 @@ pub(crate) struct Cfg<'a> { pub toolchain_override: Option, pub env_override: Option, pub dist_root_url: String, - pub notify_handler: Arc)>, + pub quiet: bool, pub current_dir: PathBuf, pub process: &'a Process, } @@ -241,7 +236,7 @@ pub(crate) struct Cfg<'a> { impl<'a> Cfg<'a> { pub(crate) fn from_env( current_dir: PathBuf, - notify_handler: Arc)>, + quiet: bool, process: &'a Process, ) -> Result { // Set up the rustup home directory @@ -302,10 +297,10 @@ impl<'a> Cfg<'a> { update_hash_dir, download_dir, tmp_cx, - notify_handler, toolchain_override: None, env_override, dist_root_url: dist_root, + quiet, current_dir, process, }; @@ -321,20 +316,6 @@ impl<'a> Cfg<'a> { Ok(cfg) } - /// construct a download configuration - pub(crate) fn download_cfg( - &'a self, - notify_handler: &'a dyn Fn(Notification<'_>), - ) -> DownloadCfg<'a> { - DownloadCfg { - dist_root: &self.dist_root_url, - tmp_cx: &self.tmp_cx, - download_dir: &self.download_dir, - notify_handler, - process: self.process, - } - } - pub(crate) fn set_profile_override(&mut self, profile: Profile) { self.profile_override = Some(profile); } @@ -1014,7 +995,7 @@ impl Debug for Cfg<'_> { toolchain_override, env_override, dist_root_url, - notify_handler: _, + quiet, current_dir, process: _, } = self; @@ -1031,6 +1012,7 @@ impl Debug for Cfg<'_> { .field("toolchain_override", toolchain_override) .field("env_override", env_override) .field("dist_root_url", dist_root_url) + .field("quiet", quiet) .field("current_dir", current_dir) .finish() } diff --git a/src/diskio/mod.rs b/src/diskio/mod.rs index a960e6f925..902813894d 100644 --- a/src/diskio/mod.rs +++ b/src/diskio/mod.rs @@ -55,6 +55,7 @@ pub(crate) mod immediate; #[cfg(test)] mod test; pub(crate) mod threaded; +use threaded::PoolReference; use std::io::{self, Write}; use std::ops::{Deref, DerefMut}; @@ -65,9 +66,8 @@ use std::{fmt::Debug, fs::OpenOptions}; use anyhow::Result; -use crate::notifications::Notification; +use crate::dist::download::Notifier; use crate::process::Process; -use threaded::PoolReference; /// Carries the implementation specific data for complete file transfers into the executor. #[derive(Debug)] @@ -443,13 +443,13 @@ pub(crate) fn create_dir>(path: P) -> io::Result<()> { /// Get the executor for disk IO. pub(crate) fn get_executor<'a>( - notify_handler: Option<&'a dyn Fn(Notification<'_>)>, + notifier: Option<&'a Notifier>, ram_budget: usize, process: &Process, ) -> anyhow::Result> { // If this gets lots of use, consider exposing via the config file. Ok(match process.io_thread_count()? { 0 | 1 => Box::new(immediate::ImmediateUnpacker::new()), - n => Box::new(threaded::Threaded::new(notify_handler, n, ram_budget)), + n => Box::new(threaded::Threaded::new(notifier, n, ram_budget)), }) } diff --git a/src/diskio/threaded.rs b/src/diskio/threaded.rs index 576bdc6b4f..e137be2108 100644 --- a/src/diskio/threaded.rs +++ b/src/diskio/threaded.rs @@ -15,7 +15,7 @@ use sharded_slab::pool::{OwnedRef, OwnedRefMut}; use tracing::debug; use super::{CompletedIo, Executor, Item, perform}; -use crate::notifications::Notification; +use crate::dist::download::{Notification, Notifier}; #[derive(Copy, Clone, Debug, Enum)] pub(crate) enum Bucket { @@ -99,7 +99,7 @@ impl fmt::Debug for Pool { pub(crate) struct Threaded<'a> { n_files: Arc, pool: threadpool::ThreadPool, - notify_handler: Option<&'a dyn Fn(Notification<'_>)>, + notifier: Option<&'a Notifier>, rx: Receiver, tx: Sender, vec_pools: EnumMap, @@ -109,7 +109,7 @@ pub(crate) struct Threaded<'a> { impl<'a> Threaded<'a> { /// Construct a new Threaded executor. pub(crate) fn new( - notify_handler: Option<&'a dyn Fn(Notification<'_>)>, + notify_handler: Option<&'a Notifier>, thread_count: usize, ram_budget: usize, ) -> Self { @@ -168,7 +168,7 @@ impl<'a> Threaded<'a> { Self { n_files: Arc::new(AtomicUsize::new(0)), pool, - notify_handler, + notifier: notify_handler, rx, tx, vec_pools, @@ -261,9 +261,9 @@ impl Executor for Threaded<'_> { // actual handling of data today, we synthesis a data buffer and // pretend to have bytes to deliver. let mut prev_files = self.n_files.load(Ordering::Relaxed); - if let Some(handler) = self.notify_handler { - handler(Notification::DownloadFinished(None)); - handler(Notification::DownloadContentLengthReceived( + if let Some(notifier) = self.notifier { + notifier.handle(Notification::DownloadFinished(None)); + notifier.handle(Notification::DownloadContentLengthReceived( prev_files as u64, None, )); @@ -282,16 +282,16 @@ impl Executor for Threaded<'_> { prev_files = current_files; current_files = self.n_files.load(Ordering::Relaxed); let step_count = prev_files - current_files; - if let Some(handler) = self.notify_handler { - handler(Notification::DownloadDataReceived( + if let Some(notifier) = self.notifier { + notifier.handle(Notification::DownloadDataReceived( &buf[0..step_count], None, )); } } self.pool.join(); - if let Some(handler) = self.notify_handler { - handler(Notification::DownloadFinished(None)); + if let Some(notifier) = self.notifier { + notifier.handle(Notification::DownloadFinished(None)); } // close the feedback channel so that blocking reads on it can // complete. send is atomic, and we know the threads completed from the diff --git a/src/dist/component/package.rs b/src/dist/component/package.rs index dcfefac91c..75db01e742 100644 --- a/src/dist/component/package.rs +++ b/src/dist/component/package.rs @@ -7,19 +7,21 @@ use std::fmt; use std::io::{self, ErrorKind as IOErrorKind, Read}; use std::mem; use std::path::{Path, PathBuf}; +use std::sync::OnceLock; use anyhow::{Context, Result, anyhow, bail}; use tar::EntryType; -use tracing::{error, warn}; +use tracing::{error, trace, warn}; use crate::diskio::{CompletedIo, Executor, FileBuffer, IO_CHUNK_SIZE, Item, Kind, get_executor}; use crate::dist::component::components::*; use crate::dist::component::transaction::*; +use crate::dist::download::Notifier; use crate::dist::temp; use crate::errors::*; -use crate::notifications::Notification; use crate::process::Process; use crate::utils; +use crate::utils::units::Size; /// The current metadata revision used by rust-installer pub(crate) const INSTALLER_VERSION: &str = "3"; @@ -199,8 +201,8 @@ fn unpack_ram( } } None => { - if let Some(h) = cx.notify_handler { - h(Notification::SetDefaultBufferSize(default_max_unpack_ram)) + if RAM_NOTICE_SHOWN.set(()).is_ok() { + trace!(size = %Size::new(default_max_unpack_ram), "unpacking components in memory"); } default_max_unpack_ram } @@ -213,6 +215,8 @@ fn unpack_ram( } } +static RAM_NOTICE_SHOWN: OnceLock<()> = OnceLock::new(); + /// Handle the async result of io operations /// Replaces op.result with Ok(()) fn filter_result(op: &mut CompletedIo) -> io::Result<()> { @@ -296,8 +300,7 @@ fn unpack_without_first_dir( } }; let unpack_ram = unpack_ram(IO_CHUNK_SIZE, effective_max_ram, cx); - let mut io_executor: Box = - get_executor(cx.notify_handler, unpack_ram, cx.process)?; + let mut io_executor: Box = get_executor(cx.notifier, unpack_ram, cx.process)?; let mut directories: HashMap = HashMap::new(); // Path is presumed to exist. Call it a precondition. @@ -631,6 +634,6 @@ impl Package for TarZStdPackage { pub(crate) struct PackageContext<'a> { pub(crate) tmp_cx: &'a temp::Context, - pub(crate) notify_handler: Option<&'a dyn Fn(Notification<'_>)>, + pub(crate) notifier: Option<&'a Notifier>, pub(crate) process: &'a Process, } diff --git a/src/dist/download.rs b/src/dist/download.rs index e3918ae9ed..e6061c3249 100644 --- a/src/dist/download.rs +++ b/src/dist/download.rs @@ -1,45 +1,43 @@ +use std::collections::HashMap; use std::fs; use std::ops; use std::path::{Path, PathBuf}; +use std::sync::Mutex; +use std::time::{Duration, Instant}; use anyhow::{Context, Result, anyhow}; +use indicatif::{MultiProgress, ProgressBar, ProgressDrawTarget, ProgressStyle}; use sha2::{Digest, Sha256}; -use tracing::debug; -use tracing::warn; +use tracing::{debug, warn}; use url::Url; +use crate::config::Cfg; use crate::dist::temp; -use crate::download::download_file; -use crate::download::download_file_with_resume; +use crate::download::{download_file, download_file_with_resume}; use crate::errors::*; -use crate::notifications::Notification; use crate::process::Process; use crate::utils; const UPDATE_HASH_LEN: usize = 20; -#[derive(Copy, Clone)] pub struct DownloadCfg<'a> { - pub dist_root: &'a str, pub tmp_cx: &'a temp::Context, pub download_dir: &'a PathBuf, - pub(crate) notify_handler: &'a dyn Fn(Notification<'_>), + pub(crate) notifier: Notifier, pub process: &'a Process, } -pub(crate) struct File { - path: PathBuf, -} - -impl ops::Deref for File { - type Target = Path; - - fn deref(&self) -> &Path { - self.path.as_path() +impl<'a> DownloadCfg<'a> { + /// construct a download configuration + pub(crate) fn new(cfg: &'a Cfg<'a>) -> Self { + DownloadCfg { + tmp_cx: &cfg.tmp_cx, + download_dir: &cfg.download_dir, + notifier: Notifier::new(cfg.quiet, cfg.process), + process: cfg.process, + } } -} -impl<'a> DownloadCfg<'a> { /// Downloads a file and validates its hash. Resumes interrupted downloads. /// Partial downloads are stored in `self.download_dir`, keyed by hash. If the /// target file already exists, then the hash is checked and it is returned @@ -49,13 +47,13 @@ impl<'a> DownloadCfg<'a> { let target_file = self.download_dir.join(Path::new(hash)); if target_file.exists() { - let cached_result = file_hash(&target_file, self.notify_handler)?; + let cached_result = file_hash(&target_file, &self.notifier)?; if hash == cached_result { - (self.notify_handler)(Notification::FileAlreadyDownloaded); + debug!("reusing previously downloaded file"); debug!(url = url.as_ref(), "checksum passed"); return Ok(File { path: target_file }); } else { - (self.notify_handler)(Notification::CachedFileChecksumFailed); + warn!("bad checksum for cached download"); fs::remove_file(&target_file).context("cleaning up previous download")?; } } @@ -78,7 +76,7 @@ impl<'a> DownloadCfg<'a> { &partial_file_path, Some(&mut hasher), true, - &|n| (self.notify_handler)(n), + &self.notifier, self.process, ) .await @@ -127,14 +125,7 @@ impl<'a> DownloadCfg<'a> { let hash_url = utils::parse_url(&(url.to_owned() + ".sha256"))?; let hash_file = self.tmp_cx.new_file()?; - download_file( - &hash_url, - &hash_file, - None, - &|n| (self.notify_handler)(n), - self.process, - ) - .await?; + download_file(&hash_url, &hash_file, None, &self.notifier, self.process).await?; utils::read_file("hash", &hash_file).map(|s| s[0..64].to_owned()) } @@ -175,14 +166,7 @@ impl<'a> DownloadCfg<'a> { let file = self.tmp_cx.new_file_with_ext("", ext)?; let mut hasher = Sha256::new(); - download_file( - &url, - &file, - Some(&mut hasher), - &|n| (self.notify_handler)(n), - self.process, - ) - .await?; + download_file(&url, &file, Some(&mut hasher), &self.notifier, self.process).await?; let actual_hash = format!("{:x}", hasher.finalize()); if hash != actual_hash { @@ -201,9 +185,178 @@ impl<'a> DownloadCfg<'a> { } } -fn file_hash(path: &Path, notify_handler: &dyn Fn(Notification<'_>)) -> Result { +pub(crate) struct Notifier { + tracker: Mutex, +} + +impl Notifier { + pub(crate) fn new(quiet: bool, process: &Process) -> Self { + Self { + tracker: Mutex::new(DownloadTracker::new(!quiet, process)), + } + } + + pub(crate) fn handle(&self, n: Notification<'_>) { + self.tracker.lock().unwrap().handle_notification(&n); + } +} + +/// Tracks download progress and displays information about it to a terminal. +/// +/// *not* safe for tracking concurrent downloads yet - it is basically undefined +/// what will happen. +pub(crate) struct DownloadTracker { + /// MultiProgress bar for the downloads. + multi_progress_bars: MultiProgress, + /// Mapping of URLs being downloaded to their corresponding progress bars. + /// The `Option` represents the instant where the download is being retried, + /// allowing us delay the reappearance of the progress bar so that the user can see + /// the message "retrying download" for at least a second. + /// Without it, the progress bar would reappear immediately, not allowing the user to + /// correctly see the message, before the progress bar starts again. + file_progress_bars: HashMap)>, +} + +impl DownloadTracker { + /// Creates a new DownloadTracker. + pub(crate) fn new(display_progress: bool, process: &Process) -> Self { + let multi_progress_bars = MultiProgress::with_draw_target(if display_progress { + process.progress_draw_target() + } else { + ProgressDrawTarget::hidden() + }); + + Self { + multi_progress_bars, + file_progress_bars: HashMap::new(), + } + } + + pub(crate) fn handle_notification(&mut self, n: &Notification<'_>) { + match *n { + Notification::DownloadContentLengthReceived(content_len, url) => { + if let Some(url) = url { + self.content_length_received(content_len, url); + } + } + Notification::DownloadDataReceived(data, url) => { + if let Some(url) = url { + self.data_received(data.len(), url); + } + } + Notification::DownloadFinished(url) => { + if let Some(url) = url { + self.download_finished(url); + } + } + Notification::DownloadFailed(url) => { + self.download_failed(url); + debug!("download failed"); + } + Notification::DownloadingComponent(component, url) => { + self.create_progress_bar(component.to_owned(), url.to_owned()); + } + Notification::RetryingDownload(url) => { + self.retrying_download(url); + } + } + } + + /// Creates a new ProgressBar for the given component. + pub(crate) fn create_progress_bar(&mut self, component: String, url: String) { + let pb = ProgressBar::hidden(); + pb.set_style( + ProgressStyle::with_template( + "{msg:>12.bold} [{bar:40}] {bytes}/{total_bytes} ({bytes_per_sec}, ETA: {eta})", + ) + .unwrap() + .progress_chars("## "), + ); + pb.set_message(component); + self.multi_progress_bars.add(pb.clone()); + self.file_progress_bars.insert(url, (pb, None)); + } + + /// Sets the length for a new ProgressBar and gives it a style. + pub(crate) fn content_length_received(&mut self, content_len: u64, url: &str) { + if let Some((pb, _)) = self.file_progress_bars.get(url) { + pb.reset(); + pb.set_length(content_len); + } + } + + /// Notifies self that data of size `len` has been received. + pub(crate) fn data_received(&mut self, len: usize, url: &str) { + let Some((pb, retry_time)) = self.file_progress_bars.get_mut(url) else { + return; + }; + pb.inc(len as u64); + if !retry_time.is_some_and(|instant| instant.elapsed() > Duration::from_secs(1)) { + return; + } + *retry_time = None; + pb.set_style( + ProgressStyle::with_template( + "{msg:>12.bold} [{bar:40}] {bytes}/{total_bytes} ({bytes_per_sec}, ETA: {eta})", + ) + .unwrap() + .progress_chars("## "), + ); + } + + /// Notifies self that the download has finished. + pub(crate) fn download_finished(&mut self, url: &str) { + let Some((pb, _)) = self.file_progress_bars.get(url) else { + return; + }; + pb.set_style( + ProgressStyle::with_template("{msg:>12.bold} downloaded {total_bytes} in {elapsed}") + .unwrap(), + ); + pb.finish(); + } + + /// Notifies self that the download has failed. + pub(crate) fn download_failed(&mut self, url: &str) { + let Some((pb, _)) = self.file_progress_bars.get(url) else { + return; + }; + pb.set_style( + ProgressStyle::with_template("{msg:>12.bold} download failed after {elapsed}") + .unwrap(), + ); + pb.finish(); + } + + /// Notifies self that the download is being retried. + pub(crate) fn retrying_download(&mut self, url: &str) { + let Some((pb, retry_time)) = self.file_progress_bars.get_mut(url) else { + return; + }; + *retry_time = Some(Instant::now()); + pb.set_style(ProgressStyle::with_template("{msg:>12.bold} retrying download").unwrap()); + } +} + +#[derive(Debug)] +pub(crate) enum Notification<'a> { + /// The URL of the download is passed as the last argument, to allow us to track concurrent downloads. + DownloadingComponent(&'a str, &'a str), + RetryingDownload(&'a str), + /// Received the Content-Length of the to-be downloaded data with + /// the respective URL of the download (for tracking concurrent downloads). + DownloadContentLengthReceived(u64, Option<&'a str>), + /// Received some data. + DownloadDataReceived(&'a [u8], Option<&'a str>), + /// Download has finished. + DownloadFinished(Option<&'a str>), + /// Download has failed. + DownloadFailed(&'a str), +} + +fn file_hash(path: &Path, notifier: &Notifier) -> Result { let mut hasher = Sha256::new(); - let mut downloaded = utils::FileReaderWithProgress::new_file(path, notify_handler)?; + let mut downloaded = utils::FileReaderWithProgress::new_file(path, notifier)?; use std::io::Read; let mut buf = vec![0; 32768]; while let Ok(n) = downloaded.read(&mut buf) { @@ -215,3 +368,15 @@ fn file_hash(path: &Path, notify_handler: &dyn Fn(Notification<'_>)) -> Result &Path { + self.path.as_path() + } +} diff --git a/src/dist/manifestation.rs b/src/dist/manifestation.rs index 14aaacbf18..00c34b0984 100644 --- a/src/dist/manifestation.rs +++ b/src/dist/manifestation.rs @@ -17,13 +17,13 @@ use crate::dist::component::{ Components, Package, PackageContext, TarGzPackage, TarXzPackage, TarZStdPackage, Transaction, }; use crate::dist::config::Config; -use crate::dist::download::{DownloadCfg, File}; +use crate::dist::download::{DownloadCfg, File, Notification}; use crate::dist::manifest::{Component, CompressionKind, HashedBinary, Manifest, TargetedPackage}; use crate::dist::prefix::InstallPrefix; +#[cfg(test)] use crate::dist::temp; use crate::dist::{DEFAULT_DIST_SERVER, Profile, TargetTriple}; use crate::errors::RustupError; -use crate::notifications::Notification; use crate::process::Process; use crate::utils; @@ -177,12 +177,12 @@ impl Manifestation { info!("downloading component(s)"); for bin in &components { - (download_cfg.notify_handler)(Notification::DownloadingComponent( - &bin.component.short_name(new_manifest), - &self.target_triple, - bin.component.target.as_ref(), - &bin.binary.url, - )); + download_cfg + .notifier + .handle(Notification::DownloadingComponent( + &bin.component.short_name(new_manifest), + &bin.binary.url, + )); } let semaphore = Arc::new(Semaphore::new(concurrent_downloads)); @@ -279,14 +279,12 @@ impl Manifestation { let cx = PackageContext { tmp_cx, - notify_handler: Some(download_cfg.notify_handler), + notifier: Some(&download_cfg.notifier), process: download_cfg.process, }; - let reader = utils::FileReaderWithProgress::new_file( - &installer_file, - download_cfg.notify_handler, - )?; + let reader = + utils::FileReaderWithProgress::new_file(&installer_file, &download_cfg.notifier)?; let package = match format { CompressionKind::GZip => &TarGzPackage::new(reader, &cx)? as &dyn Package, CompressionKind::XZ => &TarXzPackage::new(reader, &cx)?, @@ -428,9 +426,7 @@ impl Manifestation { &self, new_manifest: &[String], update_hash: Option<&Path>, - tmp_cx: &temp::Context, - notify_handler: &dyn Fn(Notification<'_>), - process: &Process, + dl_cfg: &DownloadCfg<'_>, ) -> Result> { // If there's already a v2 installation then something has gone wrong if self.read_config()?.is_some() { @@ -451,26 +447,13 @@ impl Manifestation { // Only replace once. The cost is inexpensive. let url = url .unwrap() - .replace(DEFAULT_DIST_SERVER, tmp_cx.dist_server.as_str()); - - notify_handler(Notification::DownloadingComponent( - "rust", - &self.target_triple, - Some(&self.target_triple), - &url, - )); - - use std::path::PathBuf; - let dld_dir = PathBuf::from("bogus"); - let dlcfg = DownloadCfg { - dist_root: "bogus", - download_dir: &dld_dir, - tmp_cx, - notify_handler, - process, - }; + .replace(DEFAULT_DIST_SERVER, dl_cfg.tmp_cx.dist_server.as_str()); + + dl_cfg + .notifier + .handle(Notification::DownloadingComponent("rust", &url)); - let dl = dlcfg + let dl = dl_cfg .download_and_check(&url, update_hash, ".tar.gz") .await?; if dl.is_none() { @@ -482,20 +465,20 @@ impl Manifestation { info!("installing component rust"); // Begin transaction - let mut tx = Transaction::new(prefix, tmp_cx, process); + let mut tx = Transaction::new(prefix, dl_cfg.tmp_cx, dl_cfg.process); // Uninstall components let components = self.installation.list()?; for component in components { - tx = component.uninstall(tx, process)?; + tx = component.uninstall(tx, dl_cfg.process)?; } // Install all the components in the installer - let reader = utils::FileReaderWithProgress::new_file(&installer_file, notify_handler)?; + let reader = utils::FileReaderWithProgress::new_file(&installer_file, &dl_cfg.notifier)?; let cx = PackageContext { - tmp_cx, - notify_handler: Some(notify_handler), - process, + tmp_cx: dl_cfg.tmp_cx, + notifier: Some(&dl_cfg.notifier), + process: dl_cfg.process, }; let package: &dyn Package = &TarGzPackage::new(reader, &cx)?; @@ -778,7 +761,9 @@ impl<'a> ComponentBinary<'a> { match e.downcast_ref::() { Some(RustupError::BrokenPartialFile) | Some(RustupError::DownloadingFile { .. }) => { - (download_cfg.notify_handler)(Notification::RetryingDownload(url.as_str())); + download_cfg + .notifier + .handle(Notification::RetryingDownload(url.as_str())); true } _ => false, diff --git a/src/dist/manifestation/tests.rs b/src/dist/manifestation/tests.rs index bf2090e2a5..39ad76ee34 100644 --- a/src/dist/manifestation/tests.rs +++ b/src/dist/manifestation/tests.rs @@ -3,7 +3,6 @@ #![allow(clippy::type_complexity)] use std::{ - cell::Cell, collections::HashMap, env, fs, path::{Path, PathBuf}, @@ -17,7 +16,7 @@ use url::Url; use crate::{ dist::{ DEFAULT_DIST_SERVER, Profile, TargetTriple, ToolchainDesc, - download::DownloadCfg, + download::{DownloadCfg, Notifier}, manifest::{Component, Manifest}, manifestation::{Changes, Manifestation, UpdateStatus}, prefix::InstallPrefix, @@ -25,7 +24,6 @@ use crate::{ }, download::download_file, errors::RustupError, - notifications::Notification, process::TestProcess, test::{ dist::*, @@ -471,16 +469,6 @@ impl TestContext { } } - fn default_dl_cfg(&self) -> DownloadCfg<'_> { - DownloadCfg { - dist_root: "phony", - tmp_cx: &self.tmp_cx, - download_dir: &self.download_dir, - notify_handler: &|event| println!("{event}"), - process: &self.tp.process, - } - } - // Installs or updates a toolchain from a dist server. If an initial // install then it will be installed with the default components. If // an upgrade then all the existing components will be upgraded. @@ -491,21 +479,24 @@ impl TestContext { remove: &[Component], force: bool, ) -> Result { - self.update_from_dist_with_dl_cfg(add, remove, force, &self.default_dl_cfg()) - .await - } + let dl_cfg = DownloadCfg { + tmp_cx: &self.tmp_cx, + download_dir: &self.download_dir, + notifier: Notifier::new(false, &self.tp.process), + process: &self.tp.process, + }; - async fn update_from_dist_with_dl_cfg( - &self, - add: &[Component], - remove: &[Component], - force: bool, - dl_cfg: &DownloadCfg<'_>, - ) -> Result { // Download the dist manifest and place it into the installation prefix let manifest_url = make_manifest_url(&self.url, &self.toolchain)?; let manifest_file = self.tmp_cx.new_file()?; - download_file(&manifest_url, &manifest_file, None, &|_| {}, dl_cfg.process).await?; + download_file( + &manifest_url, + &manifest_file, + None, + &dl_cfg.notifier, + dl_cfg.process, + ) + .await?; let manifest_str = utils::read_file("manifest", &manifest_file)?; let manifest = Manifest::parse(&manifest_str)?; @@ -528,7 +519,7 @@ impl TestContext { &manifest, changes, force, - dl_cfg, + &dl_cfg, &self.toolchain.manifest_name(), true, ) @@ -544,6 +535,13 @@ impl TestContext { Ok(()) } + + fn stderr_line_contains(&self, needle: &str) -> bool { + str::from_utf8(&self.tp.stderr()) + .unwrap() + .lines() + .any(|ln| ln.contains(needle)) + } } async fn initial_install(comps: Compressions) { @@ -1479,30 +1477,18 @@ fn allow_installation(prefix: &InstallPrefix) { #[tokio::test] async fn reuse_downloaded_file() { - let cx = TestContext::new(None, GZOnly); - prevent_installation(&cx.prefix); + let mut env = HashMap::default(); + env.insert("RUSTUP_LOG".to_owned(), "debug".to_owned()); + let cx = TestContext::with_env(None, GZOnly, env); + const EXPECTED_LOG: &str = "reusing previously downloaded file"; - let reuse_notification_fired = Arc::new(Cell::new(false)); - let dl_cfg = DownloadCfg { - notify_handler: &|n| { - if let Notification::FileAlreadyDownloaded = n { - reuse_notification_fired.set(true); - } - }, - ..cx.default_dl_cfg() - }; - - cx.update_from_dist_with_dl_cfg(&[], &[], false, &dl_cfg) - .await - .unwrap_err(); - assert!(!reuse_notification_fired.get()); + prevent_installation(&cx.prefix); + cx.update_from_dist(&[], &[], false).await.unwrap_err(); + assert!(!cx.stderr_line_contains(EXPECTED_LOG)); allow_installation(&cx.prefix); - cx.update_from_dist_with_dl_cfg(&[], &[], false, &dl_cfg) - .await - .unwrap(); - - assert!(reuse_notification_fired.get()); + cx.update_from_dist(&[], &[], false).await.unwrap(); + assert!(cx.stderr_line_contains(EXPECTED_LOG)); } #[tokio::test] @@ -1521,21 +1507,9 @@ async fn checks_files_hashes_before_reuse() { utils::write_file("bad previous download", &prev_download, "bad content").unwrap(); println!("wrote previous download to {}", prev_download.display()); - let noticed_bad_checksum = Arc::new(Cell::new(false)); - let dl_cfg = DownloadCfg { - notify_handler: &|n| { - if let Notification::CachedFileChecksumFailed = n { - noticed_bad_checksum.set(true); - } - }, - ..cx.default_dl_cfg() - }; - - cx.update_from_dist_with_dl_cfg(&[], &[], false, &dl_cfg) - .await - .unwrap(); - - assert!(noticed_bad_checksum.get()); + cx.update_from_dist(&[], &[], false).await.unwrap(); + const EXPECTED_LOG: &str = "bad checksum for cached download"; + assert!(cx.stderr_line_contains(EXPECTED_LOG)); } #[tokio::test] diff --git a/src/dist/mod.rs b/src/dist/mod.rs index 1d1998a59b..0f956a6c94 100644 --- a/src/dist/mod.rs +++ b/src/dist/mod.rs @@ -876,7 +876,6 @@ impl fmt::Display for Profile { } } -#[derive(Clone)] pub(crate) struct DistOptions<'a> { pub(crate) cfg: &'a Cfg<'a>, pub(crate) toolchain: &'a ToolchainDesc, @@ -968,7 +967,7 @@ pub(crate) async fn update_from_dist( let mut toolchain = opts.toolchain.clone(); let res = loop { let result = try_update_from_dist_( - opts.dl_cfg, + &opts.dl_cfg, opts.update_hash, &toolchain, match opts.exists { @@ -980,6 +979,7 @@ pub(crate) async fn update_from_dist( opts.components, opts.targets, &mut fetched, + &opts.cfg.dist_root_url, ) .await; @@ -1067,7 +1067,7 @@ pub(crate) async fn update_from_dist( #[allow(clippy::too_many_arguments)] async fn try_update_from_dist_( - download: DownloadCfg<'_>, + download: &DownloadCfg<'_>, update_hash: Option<&Path>, toolchain: &ToolchainDesc, profile: Option, @@ -1076,6 +1076,7 @@ async fn try_update_from_dist_( components: &[&str], targets: &[&str], fetched: &mut String, + dist_root: &str, ) -> Result> { let toolchain_str = toolchain.to_string(); let manifestation = Manifestation::open(prefix.clone(), toolchain.target.clone())?; @@ -1083,6 +1084,7 @@ async fn try_update_from_dist_( // TODO: Add a notification about which manifest version is going to be used info!("syncing channel updates for {toolchain_str}"); match dl_v2_manifest( + dist_root, download, // Even if manifest has not changed, we must continue to install requested components. // So if components or targets is not empty, we skip passing `update_hash` so that @@ -1152,7 +1154,7 @@ async fn try_update_from_dist_( &m, changes, force_update, - &download, + download, &toolchain.manifest_name(), true, ) @@ -1190,7 +1192,7 @@ async fn try_update_from_dist_( } // If the v2 manifest is not found then try v1 - let manifest = match dl_v1_manifest(download, toolchain).await { + let manifest = match dl_v1_manifest(dist_root, download, toolchain).await { Ok(m) => m, Err(err) => match err.downcast_ref::() { Some(RustupError::ChecksumFailed { .. }) => return Err(err), @@ -1211,13 +1213,7 @@ async fn try_update_from_dist_( }; let result = manifestation - .update_v1( - &manifest, - update_hash, - download.tmp_cx, - &download.notify_handler, - download.process, - ) + .update_v1(&manifest, update_hash, download) .await; // inspect, determine what context to add, then process afterwards. @@ -1233,11 +1229,12 @@ async fn try_update_from_dist_( } pub(crate) async fn dl_v2_manifest( - download: DownloadCfg<'_>, + dist_root: &str, + download: &DownloadCfg<'_>, update_hash: Option<&Path>, toolchain: &ToolchainDesc, ) -> Result> { - let manifest_url = toolchain.manifest_v2_url(download.dist_root, download.process); + let manifest_url = toolchain.manifest_v2_url(dist_root, download.process); match download .download_and_check(&manifest_url, update_hash, ".toml") .await @@ -1282,10 +1279,11 @@ pub(crate) async fn dl_v2_manifest( } async fn dl_v1_manifest( - download: DownloadCfg<'_>, + dist_root: &str, + download: &DownloadCfg<'_>, toolchain: &ToolchainDesc, ) -> Result> { - let root_url = toolchain.package_dir(download.dist_root); + let root_url = toolchain.package_dir(dist_root); if let Channel::Version(ver) = &toolchain.channel { // This is an explicit version. In v1 there was no manifest, @@ -1294,7 +1292,7 @@ async fn dl_v1_manifest( return Ok(vec![installer_name]); } - let manifest_url = toolchain.manifest_v1_url(download.dist_root, download.process); + let manifest_url = toolchain.manifest_v1_url(dist_root, download.process); let manifest_dl = download.download_and_check(&manifest_url, None, "").await?; let (manifest_file, _) = manifest_dl.unwrap(); let manifest_str = utils::read_file("manifest", &manifest_file)?; diff --git a/src/download/mod.rs b/src/download/mod.rs index 8bdb470099..a836927865 100644 --- a/src/download/mod.rs +++ b/src/download/mod.rs @@ -26,7 +26,11 @@ use tracing::info; use tracing::warn; use url::Url; -use crate::{errors::RustupError, notifications::Notification, process::Process}; +use crate::{ + dist::download::{Notification, Notifier}, + errors::RustupError, + process::Process, +}; #[cfg(test)] mod tests; @@ -35,10 +39,10 @@ pub(crate) async fn download_file( url: &Url, path: &Path, hasher: Option<&mut Sha256>, - notify_handler: &dyn Fn(Notification<'_>), + notifier: &Notifier, process: &Process, ) -> anyhow::Result<()> { - download_file_with_resume(url, path, hasher, false, ¬ify_handler, process).await + download_file_with_resume(url, path, hasher, false, notifier, process).await } pub(crate) async fn download_file_with_resume( @@ -46,20 +50,11 @@ pub(crate) async fn download_file_with_resume( path: &Path, hasher: Option<&mut Sha256>, resume_from_partial: bool, - notify_handler: &dyn Fn(Notification<'_>), + notifier: &Notifier, process: &Process, ) -> anyhow::Result<()> { use crate::download::DownloadError as DEK; - match download_file_( - url, - path, - hasher, - resume_from_partial, - notify_handler, - process, - ) - .await - { + match download_file_(url, path, hasher, resume_from_partial, notifier, process).await { Ok(_) => Ok(()), Err(e) => { if e.downcast_ref::().is_some() { @@ -94,7 +89,7 @@ async fn download_file_( path: &Path, hasher: Option<&mut Sha256>, resume_from_partial: bool, - notify_handler: &dyn Fn(Notification<'_>), + notifier: &Notifier, process: &Process, ) -> anyhow::Result<()> { #[cfg(any(feature = "reqwest-rustls-tls", feature = "reqwest-native-tls"))] @@ -116,13 +111,13 @@ async fn download_file_( match msg { Event::DownloadContentLengthReceived(len) => { - notify_handler(Notification::DownloadContentLengthReceived( + notifier.handle(Notification::DownloadContentLengthReceived( len, Some(url.as_str()), )); } Event::DownloadDataReceived(data) => { - notify_handler(Notification::DownloadDataReceived(data, Some(url.as_str()))); + notifier.handle(Notification::DownloadDataReceived(data, Some(url.as_str()))); } Event::ResumingPartialDownload => debug!("resuming partial download"), } @@ -224,7 +219,7 @@ async fn download_file_( .await; // The notification should only be sent if the download was successful (i.e. didn't timeout) - notify_handler(match &res { + notifier.handle(match &res { Ok(_) => Notification::DownloadFinished(Some(url.as_str())), Err(_) => Notification::DownloadFailed(url.as_str()), }); diff --git a/src/install.rs b/src/install.rs index fd2e3a8180..c921fae992 100644 --- a/src/install.rs +++ b/src/install.rs @@ -20,7 +20,6 @@ pub(crate) enum UpdateStatus { Unchanged, } -#[derive(Clone)] pub(crate) enum InstallMethod<'a> { Copy { src: &'a Path, diff --git a/src/lib.rs b/src/lib.rs index 08f855a425..14f2583a43 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -79,7 +79,6 @@ pub mod env_var; pub mod errors; mod fallback_settings; mod install; -pub mod notifications; pub mod process; mod settings; #[cfg(feature = "test")] diff --git a/src/notifications.rs b/src/notifications.rs deleted file mode 100644 index 6625a36cbe..0000000000 --- a/src/notifications.rs +++ /dev/null @@ -1,70 +0,0 @@ -use std::fmt::{self, Display}; - -use crate::dist::TargetTriple; -use crate::utils::notify::NotificationLevel; -use crate::utils::units; - -#[derive(Debug)] -pub(crate) enum Notification<'a> { - FileAlreadyDownloaded, - CachedFileChecksumFailed, - /// The URL of the download is passed as the last argument, to allow us to track concurrent downloads. - DownloadingComponent(&'a str, &'a TargetTriple, Option<&'a TargetTriple>, &'a str), - RetryingDownload(&'a str), - /// Received the Content-Length of the to-be downloaded data with - /// the respective URL of the download (for tracking concurrent downloads). - DownloadContentLengthReceived(u64, Option<&'a str>), - /// Received some data. - DownloadDataReceived(&'a [u8], Option<&'a str>), - /// Download has finished. - DownloadFinished(Option<&'a str>), - /// Download has failed. - DownloadFailed(&'a str), - /// This would make more sense as a crate::notifications::Notification - /// member, but the notification callback is already narrowed to - /// utils::notifications by the time tar unpacking is called. - SetDefaultBufferSize(usize), -} - -impl Notification<'_> { - pub(crate) fn level(&self) -> NotificationLevel { - use self::Notification::*; - match self { - FileAlreadyDownloaded => NotificationLevel::Debug, - DownloadingComponent(_, _, _, _) | RetryingDownload(_) => NotificationLevel::Info, - CachedFileChecksumFailed => NotificationLevel::Warn, - SetDefaultBufferSize(_) => NotificationLevel::Trace, - DownloadContentLengthReceived(_, _) - | DownloadDataReceived(_, _) - | DownloadFinished(_) - | DownloadFailed(_) => NotificationLevel::Debug, - } - } -} - -impl Display for Notification<'_> { - fn fmt(&self, f: &mut fmt::Formatter<'_>) -> std::result::Result<(), fmt::Error> { - use self::Notification::*; - match self { - FileAlreadyDownloaded => write!(f, "reusing previously downloaded file"), - CachedFileChecksumFailed => write!(f, "bad checksum for cached download"), - DownloadingComponent(c, h, t, _) => { - if Some(h) == t.as_ref() || t.is_none() { - write!(f, "downloading component '{c}'") - } else { - write!(f, "downloading component '{}' for '{}'", c, t.unwrap()) - } - } - RetryingDownload(url) => write!(f, "retrying download for '{url}'"), - SetDefaultBufferSize(size) => write!( - f, - "using up to {} of RAM to unpack components", - units::Size::new(*size) - ), - DownloadContentLengthReceived(len, _) => write!(f, "download size is: '{len}'"), - DownloadDataReceived(data, _) => write!(f, "received some data of size {}", data.len()), - DownloadFinished(_) => write!(f, "download finished"), - DownloadFailed(_) => write!(f, "download failed"), - } - } -} diff --git a/src/toolchain/distributable.rs b/src/toolchain/distributable.rs index 6d91312e26..d7b4a09abd 100644 --- a/src/toolchain/distributable.rs +++ b/src/toolchain/distributable.rs @@ -13,6 +13,7 @@ use crate::{ dist::{ DistOptions, PartialToolchainDesc, Profile, ToolchainDesc, config::Config, + download::DownloadCfg, manifest::{Component, ComponentStatus, Manifest}, manifestation::{Changes, Manifestation}, prefix::InstallPrefix, @@ -109,10 +110,7 @@ impl<'a> DistributableToolchain<'a> { remove_components: vec![], }; - let download_cfg = self - .toolchain - .cfg - .download_cfg(&*self.toolchain.cfg.notify_handler); + let download_cfg = DownloadCfg::new(self.toolchain.cfg); manifestation .update( &manifest, @@ -355,7 +353,7 @@ impl<'a> DistributableToolchain<'a> { toolchain, profile, update_hash, - dl_cfg: cfg.download_cfg(&|n| (cfg.notify_handler)(n)), + dl_cfg: DownloadCfg::new(cfg), force, allow_downgrade: false, exists: false, @@ -413,7 +411,7 @@ impl<'a> DistributableToolchain<'a> { toolchain: &self.desc, profile, update_hash, - dl_cfg: cfg.download_cfg(&|n| (cfg.notify_handler)(n)), + dl_cfg: DownloadCfg::new(cfg), force, allow_downgrade, exists: true, @@ -509,10 +507,7 @@ impl<'a> DistributableToolchain<'a> { remove_components: vec![component], }; - let download_cfg = self - .toolchain - .cfg - .download_cfg(&*self.toolchain.cfg.notify_handler); + let download_cfg = DownloadCfg::new(self.toolchain.cfg); manifestation .update( &manifest, @@ -528,13 +523,13 @@ impl<'a> DistributableToolchain<'a> { } pub async fn show_dist_version(&self) -> anyhow::Result> { + let dist_root = &self.toolchain.cfg.dist_root_url; let update_hash = self.toolchain.cfg.get_hash_file(&self.desc, false)?; - let download_cfg = self - .toolchain - .cfg - .download_cfg(&*self.toolchain.cfg.notify_handler); + let download_cfg = DownloadCfg::new(self.toolchain.cfg); - match crate::dist::dl_v2_manifest(download_cfg, Some(&update_hash), &self.desc).await? { + match crate::dist::dl_v2_manifest(dist_root, &download_cfg, Some(&update_hash), &self.desc) + .await? + { Some((manifest, _)) => Ok(Some(manifest.get_rust_version()?.to_string())), None => Ok(None), } diff --git a/src/utils/mod.rs b/src/utils/mod.rs index 0eef74dc39..541a872e3b 100644 --- a/src/utils/mod.rs +++ b/src/utils/mod.rs @@ -14,10 +14,10 @@ use retry::{OperationResult, retry}; use tracing::{debug, info, warn}; use url::Url; +use crate::dist::download::{Notification, Notifier}; use crate::errors::*; use crate::process::Process; -use crate::notifications::Notification; #[cfg(not(windows))] pub(crate) use crate::utils::raw::find_cmd; pub(crate) use crate::utils::raw::is_directory; @@ -447,16 +447,13 @@ pub(crate) fn delete_dir_contents_following_links(dir_path: &Path) { pub(crate) struct FileReaderWithProgress<'a> { fh: io::BufReader, - notify_handler: &'a dyn Fn(Notification<'_>), + notifier: &'a Notifier, nbytes: u64, flen: u64, } impl<'a> FileReaderWithProgress<'a> { - pub(crate) fn new_file( - path: &Path, - notify_handler: &'a dyn Fn(Notification<'_>), - ) -> Result { + pub(crate) fn new_file(path: &Path, notifier: &'a Notifier) -> Result { let fh = match File::open(path) { Ok(fh) => fh, Err(_) => { @@ -469,13 +466,13 @@ impl<'a> FileReaderWithProgress<'a> { // Inform the tracker of the file size let flen = fh.metadata()?.len(); - (notify_handler)(Notification::DownloadContentLengthReceived(flen, None)); + notifier.handle(Notification::DownloadContentLengthReceived(flen, None)); let fh = BufReader::with_capacity(8 * 1024 * 1024, fh); Ok(FileReaderWithProgress { fh, - notify_handler, + notifier, nbytes: 0, flen, }) @@ -488,13 +485,11 @@ impl io::Read for FileReaderWithProgress<'_> { Ok(nbytes) => { self.nbytes += nbytes as u64; if nbytes != 0 { - (self.notify_handler)(Notification::DownloadDataReceived( - &buf[0..nbytes], - None, - )); + self.notifier + .handle(Notification::DownloadDataReceived(&buf[0..nbytes], None)); } if (nbytes == 0) || (self.flen == self.nbytes) { - (self.notify_handler)(Notification::DownloadFinished(None)); + self.notifier.handle(Notification::DownloadFinished(None)); } Ok(nbytes) }