From ee7a99664e0d23890882584f07acbaa6aca4c86b Mon Sep 17 00:00:00 2001 From: Dirkjan Ochtman Date: Sat, 11 Oct 2025 10:22:12 +0200 Subject: [PATCH 01/18] notifications: log directly on buffer size changes --- src/cli/common.rs | 10 ---------- src/dist/component/package.rs | 10 +++++++--- src/notifications.rs | 11 ----------- 3 files changed, 7 insertions(+), 24 deletions(-) diff --git a/src/cli/common.rs b/src/cli/common.rs index bb5002cf9c..d968a29b7c 100644 --- a/src/cli/common.rs +++ b/src/cli/common.rs @@ -1,6 +1,5 @@ //! Just a dumping ground for cli stuff -use std::cell::RefCell; use std::fmt::Display; use std::fs; use std::io::{BufRead, Write}; @@ -124,14 +123,12 @@ pub(crate) fn read_line(process: &Process) -> Result { 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), } } @@ -140,13 +137,6 @@ impl Notifier { 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 { diff --git a/src/dist/component/package.rs b/src/dist/component/package.rs index dcfefac91c..80e8cef3bd 100644 --- a/src/dist/component/package.rs +++ b/src/dist/component/package.rs @@ -7,10 +7,11 @@ 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::*; @@ -20,6 +21,7 @@ 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<()> { diff --git a/src/notifications.rs b/src/notifications.rs index 6625a36cbe..fc4aea0eb5 100644 --- a/src/notifications.rs +++ b/src/notifications.rs @@ -2,7 +2,6 @@ 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> { @@ -20,10 +19,6 @@ pub(crate) enum Notification<'a> { 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<'_> { @@ -33,7 +28,6 @@ impl Notification<'_> { FileAlreadyDownloaded => NotificationLevel::Debug, DownloadingComponent(_, _, _, _) | RetryingDownload(_) => NotificationLevel::Info, CachedFileChecksumFailed => NotificationLevel::Warn, - SetDefaultBufferSize(_) => NotificationLevel::Trace, DownloadContentLengthReceived(_, _) | DownloadDataReceived(_, _) | DownloadFinished(_) @@ -56,11 +50,6 @@ impl Display for Notification<'_> { } } 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"), From df4105bbc3a629c32ae29dd3cc0743af336e0858 Mon Sep 17 00:00:00 2001 From: Dirkjan Ochtman Date: Sat, 11 Oct 2025 11:06:06 +0200 Subject: [PATCH 02/18] notifications: log directly when reusing downloaded files --- src/dist/download.rs | 2 +- src/dist/manifestation/tests.rs | 37 ++++++++++++++------------------- src/notifications.rs | 3 --- 3 files changed, 17 insertions(+), 25 deletions(-) diff --git a/src/dist/download.rs b/src/dist/download.rs index e3918ae9ed..f877302bde 100644 --- a/src/dist/download.rs +++ b/src/dist/download.rs @@ -51,7 +51,7 @@ impl<'a> DownloadCfg<'a> { if target_file.exists() { let cached_result = file_hash(&target_file, self.notify_handler)?; 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 { diff --git a/src/dist/manifestation/tests.rs b/src/dist/manifestation/tests.rs index bf2090e2a5..441b2b9ef0 100644 --- a/src/dist/manifestation/tests.rs +++ b/src/dist/manifestation/tests.rs @@ -544,6 +544,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 +1486,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] diff --git a/src/notifications.rs b/src/notifications.rs index fc4aea0eb5..c46a18cc44 100644 --- a/src/notifications.rs +++ b/src/notifications.rs @@ -5,7 +5,6 @@ use crate::utils::notify::NotificationLevel; #[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), @@ -25,7 +24,6 @@ impl Notification<'_> { pub(crate) fn level(&self) -> NotificationLevel { use self::Notification::*; match self { - FileAlreadyDownloaded => NotificationLevel::Debug, DownloadingComponent(_, _, _, _) | RetryingDownload(_) => NotificationLevel::Info, CachedFileChecksumFailed => NotificationLevel::Warn, DownloadContentLengthReceived(_, _) @@ -40,7 +38,6 @@ 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() { From 64f86b1405d042d4bc00798915767b125f38313e Mon Sep 17 00:00:00 2001 From: Dirkjan Ochtman Date: Sat, 11 Oct 2025 11:10:01 +0200 Subject: [PATCH 03/18] notifications: log directly on bad download checksums --- src/cli/download_tracker.rs | 1 - src/dist/download.rs | 2 +- src/dist/manifestation/tests.rs | 20 +++----------------- src/notifications.rs | 3 --- 4 files changed, 4 insertions(+), 22 deletions(-) diff --git a/src/cli/download_tracker.rs b/src/cli/download_tracker.rs index a2105dd048..c1a852caa3 100644 --- a/src/cli/download_tracker.rs +++ b/src/cli/download_tracker.rs @@ -68,7 +68,6 @@ impl DownloadTracker { self.retrying_download(url); true } - _ => false, } } diff --git a/src/dist/download.rs b/src/dist/download.rs index f877302bde..5e11004d7c 100644 --- a/src/dist/download.rs +++ b/src/dist/download.rs @@ -55,7 +55,7 @@ impl<'a> DownloadCfg<'a> { 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")?; } } diff --git a/src/dist/manifestation/tests.rs b/src/dist/manifestation/tests.rs index 441b2b9ef0..5bb8f289e0 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}, @@ -25,7 +24,6 @@ use crate::{ }, download::download_file, errors::RustupError, - notifications::Notification, process::TestProcess, test::{ dist::*, @@ -1516,21 +1514,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/notifications.rs b/src/notifications.rs index c46a18cc44..41a025f8d0 100644 --- a/src/notifications.rs +++ b/src/notifications.rs @@ -5,7 +5,6 @@ use crate::utils::notify::NotificationLevel; #[derive(Debug)] pub(crate) enum Notification<'a> { - 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), @@ -25,7 +24,6 @@ impl Notification<'_> { use self::Notification::*; match self { DownloadingComponent(_, _, _, _) | RetryingDownload(_) => NotificationLevel::Info, - CachedFileChecksumFailed => NotificationLevel::Warn, DownloadContentLengthReceived(_, _) | DownloadDataReceived(_, _) | DownloadFinished(_) @@ -38,7 +36,6 @@ impl Display for Notification<'_> { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> std::result::Result<(), fmt::Error> { use self::Notification::*; match self { - 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}'") From bc1e44c9bcc649b2ab90fa85c1ae32f2ad39f906 Mon Sep 17 00:00:00 2001 From: Dirkjan Ochtman Date: Sat, 11 Oct 2025 11:17:22 +0200 Subject: [PATCH 04/18] notifications: log directly from DownloadTracker --- src/cli/common.rs | 29 +++-------------------------- src/cli/download_tracker.rs | 10 +++------- src/notifications.rs | 14 -------------- 3 files changed, 6 insertions(+), 47 deletions(-) diff --git a/src/cli/common.rs b/src/cli/common.rs index d968a29b7c..66f9741e3e 100644 --- a/src/cli/common.rs +++ b/src/cli/common.rs @@ -10,7 +10,7 @@ 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::{ @@ -22,7 +22,7 @@ use crate::{ 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"; @@ -133,30 +133,7 @@ impl Notifier { } pub(super) fn handle(&self, n: Notification<'_>) { - if self.tracker.lock().unwrap().handle_notification(&n) { - return; - } - - 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); - } - } - } + self.tracker.lock().unwrap().handle_notification(&n); } } diff --git a/src/cli/download_tracker.rs b/src/cli/download_tracker.rs index c1a852caa3..b43badc2d7 100644 --- a/src/cli/download_tracker.rs +++ b/src/cli/download_tracker.rs @@ -1,6 +1,7 @@ use indicatif::{MultiProgress, ProgressBar, ProgressDrawTarget, ProgressStyle}; use std::collections::HashMap; use std::time::{Duration, Instant}; +use tracing::debug; use crate::notifications::Notification; use crate::process::Process; @@ -36,37 +37,32 @@ impl DownloadTracker { } } - pub(crate) fn handle_notification(&mut self, n: &Notification<'_>) -> bool { + 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); } - 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 + debug!("download failed"); } Notification::DownloadingComponent(component, _, _, url) => { self.create_progress_bar(component.to_owned(), url.to_owned()); - true } Notification::RetryingDownload(url) => { self.retrying_download(url); - true } } } diff --git a/src/notifications.rs b/src/notifications.rs index 41a025f8d0..a2cf911d29 100644 --- a/src/notifications.rs +++ b/src/notifications.rs @@ -1,7 +1,6 @@ use std::fmt::{self, Display}; use crate::dist::TargetTriple; -use crate::utils::notify::NotificationLevel; #[derive(Debug)] pub(crate) enum Notification<'a> { @@ -19,19 +18,6 @@ pub(crate) enum Notification<'a> { DownloadFailed(&'a str), } -impl Notification<'_> { - pub(crate) fn level(&self) -> NotificationLevel { - use self::Notification::*; - match self { - DownloadingComponent(_, _, _, _) | RetryingDownload(_) => NotificationLevel::Info, - 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::*; From b9be191712c01c65b4d68b52824a0319d7d85da5 Mon Sep 17 00:00:00 2001 From: Dirkjan Ochtman Date: Sat, 11 Oct 2025 11:17:55 +0200 Subject: [PATCH 05/18] cli: rename DownloadTracker::new_with_display_progress() to new() --- src/cli/common.rs | 2 +- src/cli/download_tracker.rs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/cli/common.rs b/src/cli/common.rs index 66f9741e3e..57a410ab36 100644 --- a/src/cli/common.rs +++ b/src/cli/common.rs @@ -128,7 +128,7 @@ pub(super) struct Notifier { impl Notifier { pub(super) fn new(quiet: bool, process: &Process) -> Self { Self { - tracker: Mutex::new(DownloadTracker::new_with_display_progress(!quiet, process)), + tracker: Mutex::new(DownloadTracker::new(!quiet, process)), } } diff --git a/src/cli/download_tracker.rs b/src/cli/download_tracker.rs index b43badc2d7..819c22d70a 100644 --- a/src/cli/download_tracker.rs +++ b/src/cli/download_tracker.rs @@ -24,7 +24,7 @@ pub(crate) struct DownloadTracker { impl DownloadTracker { /// Creates a new DownloadTracker. - pub(crate) fn new_with_display_progress(display_progress: bool, process: &Process) -> Self { + 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 { From 48420a3da82926168b3a69e9442a8d5842920d0a Mon Sep 17 00:00:00 2001 From: Dirkjan Ochtman Date: Sat, 11 Oct 2025 11:22:50 +0200 Subject: [PATCH 06/18] config: discard pointless method argument --- src/config.rs | 7 ++----- src/toolchain/distributable.rs | 19 +++++-------------- 2 files changed, 7 insertions(+), 19 deletions(-) diff --git a/src/config.rs b/src/config.rs index 9f238038d0..97391562eb 100644 --- a/src/config.rs +++ b/src/config.rs @@ -322,15 +322,12 @@ impl<'a> Cfg<'a> { } /// construct a download configuration - pub(crate) fn download_cfg( - &'a self, - notify_handler: &'a dyn Fn(Notification<'_>), - ) -> DownloadCfg<'a> { + pub(crate) fn download_cfg(&'a self) -> DownloadCfg<'a> { DownloadCfg { dist_root: &self.dist_root_url, tmp_cx: &self.tmp_cx, download_dir: &self.download_dir, - notify_handler, + notify_handler: &*self.notify_handler, process: self.process, } } diff --git a/src/toolchain/distributable.rs b/src/toolchain/distributable.rs index 6d91312e26..8f1bb6b822 100644 --- a/src/toolchain/distributable.rs +++ b/src/toolchain/distributable.rs @@ -109,10 +109,7 @@ impl<'a> DistributableToolchain<'a> { remove_components: vec![], }; - let download_cfg = self - .toolchain - .cfg - .download_cfg(&*self.toolchain.cfg.notify_handler); + let download_cfg = self.toolchain.cfg.download_cfg(); manifestation .update( &manifest, @@ -355,7 +352,7 @@ impl<'a> DistributableToolchain<'a> { toolchain, profile, update_hash, - dl_cfg: cfg.download_cfg(&|n| (cfg.notify_handler)(n)), + dl_cfg: cfg.download_cfg(), force, allow_downgrade: false, exists: false, @@ -413,7 +410,7 @@ impl<'a> DistributableToolchain<'a> { toolchain: &self.desc, profile, update_hash, - dl_cfg: cfg.download_cfg(&|n| (cfg.notify_handler)(n)), + dl_cfg: cfg.download_cfg(), force, allow_downgrade, exists: true, @@ -509,10 +506,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 = self.toolchain.cfg.download_cfg(); manifestation .update( &manifest, @@ -529,10 +523,7 @@ impl<'a> DistributableToolchain<'a> { pub async fn show_dist_version(&self) -> anyhow::Result> { 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 = self.toolchain.cfg.download_cfg(); match crate::dist::dl_v2_manifest(download_cfg, Some(&update_hash), &self.desc).await? { Some((manifest, _)) => Ok(Some(manifest.get_rust_version()?.to_string())), From 0ae365613f1affd6dac6e26f0bc90acd1ae37e69 Mon Sep 17 00:00:00 2001 From: Dirkjan Ochtman Date: Sat, 11 Oct 2025 11:28:16 +0200 Subject: [PATCH 07/18] download: extract DownloadCfg initialization from Cfg --- src/config.rs | 16 +--------------- src/dist/download.rs | 12 ++++++++++++ src/toolchain/distributable.rs | 11 ++++++----- 3 files changed, 19 insertions(+), 20 deletions(-) diff --git a/src/config.rs b/src/config.rs index 97391562eb..dbd4de1221 100644 --- a/src/config.rs +++ b/src/config.rs @@ -13,10 +13,7 @@ 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, @@ -321,17 +318,6 @@ impl<'a> Cfg<'a> { Ok(cfg) } - /// construct a download configuration - pub(crate) fn download_cfg(&'a self) -> DownloadCfg<'a> { - DownloadCfg { - dist_root: &self.dist_root_url, - tmp_cx: &self.tmp_cx, - download_dir: &self.download_dir, - notify_handler: &*self.notify_handler, - process: self.process, - } - } - pub(crate) fn set_profile_override(&mut self, profile: Profile) { self.profile_override = Some(profile); } diff --git a/src/dist/download.rs b/src/dist/download.rs index 5e11004d7c..5e535f6df6 100644 --- a/src/dist/download.rs +++ b/src/dist/download.rs @@ -8,6 +8,7 @@ use tracing::debug; use tracing::warn; use url::Url; +use crate::config::Cfg; use crate::dist::temp; use crate::download::download_file; use crate::download::download_file_with_resume; @@ -40,6 +41,17 @@ impl ops::Deref for File { } impl<'a> DownloadCfg<'a> { + /// construct a download configuration + pub(crate) fn new(cfg: &'a Cfg<'a>) -> Self { + DownloadCfg { + dist_root: &cfg.dist_root_url, + tmp_cx: &cfg.tmp_cx, + download_dir: &cfg.download_dir, + notify_handler: &*cfg.notify_handler, + process: cfg.process, + } + } + /// 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 diff --git a/src/toolchain/distributable.rs b/src/toolchain/distributable.rs index 8f1bb6b822..7db5a08e43 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,7 +110,7 @@ impl<'a> DistributableToolchain<'a> { remove_components: vec![], }; - let download_cfg = self.toolchain.cfg.download_cfg(); + let download_cfg = DownloadCfg::new(self.toolchain.cfg); manifestation .update( &manifest, @@ -352,7 +353,7 @@ impl<'a> DistributableToolchain<'a> { toolchain, profile, update_hash, - dl_cfg: cfg.download_cfg(), + dl_cfg: DownloadCfg::new(cfg), force, allow_downgrade: false, exists: false, @@ -410,7 +411,7 @@ impl<'a> DistributableToolchain<'a> { toolchain: &self.desc, profile, update_hash, - dl_cfg: cfg.download_cfg(), + dl_cfg: DownloadCfg::new(cfg), force, allow_downgrade, exists: true, @@ -506,7 +507,7 @@ impl<'a> DistributableToolchain<'a> { remove_components: vec![component], }; - let download_cfg = self.toolchain.cfg.download_cfg(); + let download_cfg = DownloadCfg::new(self.toolchain.cfg); manifestation .update( &manifest, @@ -523,7 +524,7 @@ impl<'a> DistributableToolchain<'a> { pub async fn show_dist_version(&self) -> anyhow::Result> { let update_hash = self.toolchain.cfg.get_hash_file(&self.desc, false)?; - let download_cfg = self.toolchain.cfg.download_cfg(); + let download_cfg = DownloadCfg::new(self.toolchain.cfg); match crate::dist::dl_v2_manifest(download_cfg, Some(&update_hash), &self.desc).await? { Some((manifest, _)) => Ok(Some(manifest.get_rust_version()?.to_string())), From 518b1b26931075802bb56ad4a8aad48ea702db9b Mon Sep 17 00:00:00 2001 From: Dirkjan Ochtman Date: Sat, 11 Oct 2025 11:29:06 +0200 Subject: [PATCH 08/18] download: move File items down --- src/dist/download.rs | 24 ++++++++++++------------ 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/src/dist/download.rs b/src/dist/download.rs index 5e535f6df6..2c1416a53c 100644 --- a/src/dist/download.rs +++ b/src/dist/download.rs @@ -28,18 +28,6 @@ pub struct DownloadCfg<'a> { 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 { @@ -227,3 +215,15 @@ fn file_hash(path: &Path, notify_handler: &dyn Fn(Notification<'_>)) -> Result &Path { + self.path.as_path() + } +} From 0284afdcf69c1d29274d812acbee0bdaf2b44dd0 Mon Sep 17 00:00:00 2001 From: Dirkjan Ochtman Date: Sat, 11 Oct 2025 11:34:32 +0200 Subject: [PATCH 09/18] dist: inline DownloadCfg test setup --- src/dist/manifestation/tests.rs | 29 ++++++++--------------------- 1 file changed, 8 insertions(+), 21 deletions(-) diff --git a/src/dist/manifestation/tests.rs b/src/dist/manifestation/tests.rs index 5bb8f289e0..fdde658360 100644 --- a/src/dist/manifestation/tests.rs +++ b/src/dist/manifestation/tests.rs @@ -469,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. @@ -489,17 +479,14 @@ 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 { + dist_root: "phony", + tmp_cx: &self.tmp_cx, + download_dir: &self.download_dir, + notify_handler: &|event| println!("{event}"), + 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()?; @@ -526,7 +513,7 @@ impl TestContext { &manifest, changes, force, - dl_cfg, + &dl_cfg, &self.toolchain.manifest_name(), true, ) From b59416f9fc33bd28001594cc13d8f37b87e99f67 Mon Sep 17 00:00:00 2001 From: Dirkjan Ochtman Date: Sat, 11 Oct 2025 11:47:05 +0200 Subject: [PATCH 10/18] dist: move Notifier and DownloadTracker into dist::download --- src/cli.rs | 1 - src/cli/common.rs | 21 +---- src/cli/download_tracker.rs | 144 ------------------------------- src/dist/download.rs | 163 +++++++++++++++++++++++++++++++++++- 4 files changed, 161 insertions(+), 168 deletions(-) delete mode 100644 src/cli/download_tracker.rs 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 57a410ab36..7c834f671e 100644 --- a/src/cli/common.rs +++ b/src/cli/common.rs @@ -4,7 +4,7 @@ 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::{Arc, LazyLock}; use std::{cmp, env}; use anyhow::{Context, Result, anyhow}; @@ -13,13 +13,12 @@ use termcolor::Color; use tracing::{error, info, warn}; use tracing_subscriber::{EnvFilter, Registry, reload::Handle}; +use crate::dist::download::Notifier; 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, @@ -121,22 +120,6 @@ pub(crate) fn read_line(process: &Process) -> Result { .context("unable to read from stdin for confirmation") } -pub(super) struct Notifier { - tracker: Mutex, -} - -impl Notifier { - pub(super) fn new(quiet: bool, process: &Process) -> Self { - Self { - tracker: Mutex::new(DownloadTracker::new(!quiet, process)), - } - } - - pub(super) fn handle(&self, n: Notification<'_>) { - self.tracker.lock().unwrap().handle_notification(&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); diff --git a/src/cli/download_tracker.rs b/src/cli/download_tracker.rs deleted file mode 100644 index 819c22d70a..0000000000 --- a/src/cli/download_tracker.rs +++ /dev/null @@ -1,144 +0,0 @@ -use indicatif::{MultiProgress, ProgressBar, ProgressDrawTarget, ProgressStyle}; -use std::collections::HashMap; -use std::time::{Duration, Instant}; -use tracing::debug; - -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(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()); - } -} diff --git a/src/dist/download.rs b/src/dist/download.rs index 2c1416a53c..0d64fa708b 100644 --- a/src/dist/download.rs +++ b/src/dist/download.rs @@ -1,17 +1,19 @@ +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; @@ -201,6 +203,159 @@ impl<'a> DownloadCfg<'a> { } } +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()); + } +} + fn file_hash(path: &Path, notify_handler: &dyn Fn(Notification<'_>)) -> Result { let mut hasher = Sha256::new(); let mut downloaded = utils::FileReaderWithProgress::new_file(path, notify_handler)?; From 634c96e0e9bebe981bc8c031fb8f53261c92f5d0 Mon Sep 17 00:00:00 2001 From: Dirkjan Ochtman Date: Sat, 11 Oct 2025 11:50:27 +0200 Subject: [PATCH 11/18] dist: drop Copy derive from DownloadCfg --- src/dist/download.rs | 2 +- src/dist/mod.rs | 10 +++++----- src/toolchain/distributable.rs | 2 +- 3 files changed, 7 insertions(+), 7 deletions(-) diff --git a/src/dist/download.rs b/src/dist/download.rs index 0d64fa708b..5be44c6733 100644 --- a/src/dist/download.rs +++ b/src/dist/download.rs @@ -21,7 +21,7 @@ use crate::utils; const UPDATE_HASH_LEN: usize = 20; -#[derive(Copy, Clone)] +#[derive(Clone)] pub struct DownloadCfg<'a> { pub dist_root: &'a str, pub tmp_cx: &'a temp::Context, diff --git a/src/dist/mod.rs b/src/dist/mod.rs index 1d1998a59b..37c009f2b9 100644 --- a/src/dist/mod.rs +++ b/src/dist/mod.rs @@ -968,7 +968,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 { @@ -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, @@ -1152,7 +1152,7 @@ async fn try_update_from_dist_( &m, changes, force_update, - &download, + download, &toolchain.manifest_name(), true, ) @@ -1233,7 +1233,7 @@ async fn try_update_from_dist_( } pub(crate) async fn dl_v2_manifest( - download: DownloadCfg<'_>, + download: &DownloadCfg<'_>, update_hash: Option<&Path>, toolchain: &ToolchainDesc, ) -> Result> { @@ -1282,7 +1282,7 @@ pub(crate) async fn dl_v2_manifest( } async fn dl_v1_manifest( - download: DownloadCfg<'_>, + download: &DownloadCfg<'_>, toolchain: &ToolchainDesc, ) -> Result> { let root_url = toolchain.package_dir(download.dist_root); diff --git a/src/toolchain/distributable.rs b/src/toolchain/distributable.rs index 7db5a08e43..9fde8d2a27 100644 --- a/src/toolchain/distributable.rs +++ b/src/toolchain/distributable.rs @@ -526,7 +526,7 @@ impl<'a> DistributableToolchain<'a> { let update_hash = self.toolchain.cfg.get_hash_file(&self.desc, false)?; 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(&download_cfg, Some(&update_hash), &self.desc).await? { Some((manifest, _)) => Ok(Some(manifest.get_rust_version()?.to_string())), None => Ok(None), } From 70ddfdf1179b19552d16caac375a679bf5877c08 Mon Sep 17 00:00:00 2001 From: Dirkjan Ochtman Date: Sat, 11 Oct 2025 11:51:18 +0200 Subject: [PATCH 12/18] dist: drop unused Clone derives --- src/dist/download.rs | 1 - src/dist/mod.rs | 1 - src/install.rs | 1 - 3 files changed, 3 deletions(-) diff --git a/src/dist/download.rs b/src/dist/download.rs index 5be44c6733..1df88fc8b1 100644 --- a/src/dist/download.rs +++ b/src/dist/download.rs @@ -21,7 +21,6 @@ use crate::utils; const UPDATE_HASH_LEN: usize = 20; -#[derive(Clone)] pub struct DownloadCfg<'a> { pub dist_root: &'a str, pub tmp_cx: &'a temp::Context, diff --git a/src/dist/mod.rs b/src/dist/mod.rs index 37c009f2b9..0f90a97414 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, 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, From abb5c81b452f3af51f8405be3fe9be5f12212afc Mon Sep 17 00:00:00 2001 From: Dirkjan Ochtman Date: Sat, 11 Oct 2025 12:42:11 +0200 Subject: [PATCH 13/18] dist: move dist_root out of DownloadCfg --- src/dist/download.rs | 2 -- src/dist/manifestation.rs | 1 - src/dist/manifestation/tests.rs | 1 - src/dist/mod.rs | 13 +++++++++---- src/toolchain/distributable.rs | 5 ++++- 5 files changed, 13 insertions(+), 9 deletions(-) diff --git a/src/dist/download.rs b/src/dist/download.rs index 1df88fc8b1..07ccc01810 100644 --- a/src/dist/download.rs +++ b/src/dist/download.rs @@ -22,7 +22,6 @@ use crate::utils; const UPDATE_HASH_LEN: usize = 20; 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<'_>), @@ -33,7 +32,6 @@ impl<'a> DownloadCfg<'a> { /// construct a download configuration pub(crate) fn new(cfg: &'a Cfg<'a>) -> Self { DownloadCfg { - dist_root: &cfg.dist_root_url, tmp_cx: &cfg.tmp_cx, download_dir: &cfg.download_dir, notify_handler: &*cfg.notify_handler, diff --git a/src/dist/manifestation.rs b/src/dist/manifestation.rs index 14aaacbf18..798fc9be73 100644 --- a/src/dist/manifestation.rs +++ b/src/dist/manifestation.rs @@ -463,7 +463,6 @@ impl Manifestation { use std::path::PathBuf; let dld_dir = PathBuf::from("bogus"); let dlcfg = DownloadCfg { - dist_root: "bogus", download_dir: &dld_dir, tmp_cx, notify_handler, diff --git a/src/dist/manifestation/tests.rs b/src/dist/manifestation/tests.rs index fdde658360..e37516d2c6 100644 --- a/src/dist/manifestation/tests.rs +++ b/src/dist/manifestation/tests.rs @@ -480,7 +480,6 @@ impl TestContext { force: bool, ) -> Result { let dl_cfg = DownloadCfg { - dist_root: "phony", tmp_cx: &self.tmp_cx, download_dir: &self.download_dir, notify_handler: &|event| println!("{event}"), diff --git a/src/dist/mod.rs b/src/dist/mod.rs index 0f90a97414..ed49b9b15b 100644 --- a/src/dist/mod.rs +++ b/src/dist/mod.rs @@ -979,6 +979,7 @@ pub(crate) async fn update_from_dist( opts.components, opts.targets, &mut fetched, + &opts.cfg.dist_root_url, ) .await; @@ -1075,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())?; @@ -1082,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 @@ -1189,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), @@ -1232,11 +1235,12 @@ async fn try_update_from_dist_( } pub(crate) async fn dl_v2_manifest( + 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 @@ -1281,10 +1285,11 @@ pub(crate) async fn dl_v2_manifest( } async fn dl_v1_manifest( + 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, @@ -1293,7 +1298,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/toolchain/distributable.rs b/src/toolchain/distributable.rs index 9fde8d2a27..d7b4a09abd 100644 --- a/src/toolchain/distributable.rs +++ b/src/toolchain/distributable.rs @@ -523,10 +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 = 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), } From 6bdb9107d153dcbf5b213cbe6b720fd00633684c Mon Sep 17 00:00:00 2001 From: Dirkjan Ochtman Date: Sat, 11 Oct 2025 12:47:29 +0200 Subject: [PATCH 14/18] dist: reuse existing DownloadCfg in update_v1() --- src/dist/manifestation.rs | 33 ++++++++++++--------------------- src/dist/mod.rs | 8 +------- 2 files changed, 13 insertions(+), 28 deletions(-) diff --git a/src/dist/manifestation.rs b/src/dist/manifestation.rs index 798fc9be73..98c72ef146 100644 --- a/src/dist/manifestation.rs +++ b/src/dist/manifestation.rs @@ -20,6 +20,7 @@ use crate::dist::config::Config; use crate::dist::download::{DownloadCfg, File}; 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; @@ -428,9 +429,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,25 +450,16 @@ impl Manifestation { // Only replace once. The cost is inexpensive. let url = url .unwrap() - .replace(DEFAULT_DIST_SERVER, tmp_cx.dist_server.as_str()); + .replace(DEFAULT_DIST_SERVER, dl_cfg.tmp_cx.dist_server.as_str()); - notify_handler(Notification::DownloadingComponent( + (dl_cfg.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 { - download_dir: &dld_dir, - tmp_cx, - notify_handler, - process, - }; - - let dl = dlcfg + let dl = dl_cfg .download_and_check(&url, update_hash, ".tar.gz") .await?; if dl.is_none() { @@ -481,20 +471,21 @@ 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.notify_handler)?; let cx = PackageContext { - tmp_cx, - notify_handler: Some(notify_handler), - process, + tmp_cx: dl_cfg.tmp_cx, + notify_handler: Some(dl_cfg.notify_handler), + process: dl_cfg.process, }; let package: &dyn Package = &TarGzPackage::new(reader, &cx)?; diff --git a/src/dist/mod.rs b/src/dist/mod.rs index ed49b9b15b..0f956a6c94 100644 --- a/src/dist/mod.rs +++ b/src/dist/mod.rs @@ -1213,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. From b7b9d81223bc4b66c9cb04466673407956d8c659 Mon Sep 17 00:00:00 2001 From: Dirkjan Ochtman Date: Sun, 12 Oct 2025 10:31:49 +0200 Subject: [PATCH 15/18] cli: build Cfg earlier in setup mode --- src/cli/self_update.rs | 71 +++++++++++++++++++----------------------- src/cli/setup_mode.rs | 3 +- 2 files changed, 34 insertions(+), 40 deletions(-) diff --git a/src/cli/self_update.rs b/src/cli/self_update.rs index ea421f7b3f..39fdb7007b 100644 --- a/src/cli/self_update.rs +++ b/src/cli/self_update.rs @@ -508,50 +508,49 @@ fn canonical_cargo_home(process: &Process) -> 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?; 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 +560,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 @@ -583,7 +582,7 @@ pub(crate) async fn install( 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 +595,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,7 +613,7 @@ 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 { @@ -911,24 +910,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 +931,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 +958,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(()) } 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 } From df12b1bb0b685b7d6ed66d779c59f21d3316d0c7 Mon Sep 17 00:00:00 2001 From: Dirkjan Ochtman Date: Sat, 11 Oct 2025 12:49:23 +0200 Subject: [PATCH 16/18] dist: move Notifier into DownloadCfg --- src/cli/common.rs | 6 ++-- src/cli/rustup_mode.rs | 12 ++++--- src/cli/self_update.rs | 55 +++++++++++++++++++++------------ src/cli/self_update/windows.rs | 51 +++++++++++++----------------- src/config.rs | 11 +++---- src/diskio/mod.rs | 8 ++--- src/diskio/threaded.rs | 21 +++++++------ src/dist/component/package.rs | 7 ++--- src/dist/download.rs | 28 +++++------------ src/dist/manifestation.rs | 33 ++++++++++---------- src/dist/manifestation/tests.rs | 13 ++++++-- src/download/mod.rs | 29 +++++++---------- src/utils/mod.rs | 20 +++++------- 13 files changed, 142 insertions(+), 152 deletions(-) diff --git a/src/cli/common.rs b/src/cli/common.rs index 7c834f671e..46e722ad29 100644 --- a/src/cli/common.rs +++ b/src/cli/common.rs @@ -4,7 +4,7 @@ use std::fmt::Display; use std::fs; use std::io::{BufRead, Write}; use std::path::{Path, PathBuf}; -use std::sync::{Arc, LazyLock}; +use std::sync::LazyLock; use std::{cmp, env}; use anyhow::{Context, Result, anyhow}; @@ -13,7 +13,6 @@ use termcolor::Color; use tracing::{error, info, warn}; use tracing_subscriber::{EnvFilter, Registry, reload::Handle}; -use crate::dist::download::Notifier; use crate::{ config::Cfg, dist::{TargetTriple, ToolchainDesc}, @@ -122,8 +121,7 @@ pub(crate) fn read_line(process: &Process) -> Result { #[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/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 Result { +pub(crate) async fn self_update(dl_cfg: &DownloadCfg<'_>) -> Result { match self_update_permitted(false)? { SelfUpdatePermission::HardFail => { error!("Unable to self-update. STOP"); @@ -1116,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)) @@ -1167,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"); @@ -1220,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}")); @@ -1243,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 @@ -1274,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)?; @@ -1282,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() @@ -1293,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")?; @@ -1341,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/config.rs b/src/config.rs index dbd4de1221..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; @@ -17,7 +16,6 @@ use crate::{ errors::RustupError, fallback_settings::FallbackSettings, install::UpdateStatus, - notifications::*, process::Process, settings::{MetadataVersion, Settings, SettingsFile}, toolchain::{ @@ -230,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, } @@ -238,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 @@ -299,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, }; @@ -997,7 +995,7 @@ impl Debug for Cfg<'_> { toolchain_override, env_override, dist_root_url, - notify_handler: _, + quiet, current_dir, process: _, } = self; @@ -1014,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..afe6fe8e8f 100644 --- a/src/diskio/threaded.rs +++ b/src/diskio/threaded.rs @@ -15,6 +15,7 @@ use sharded_slab::pool::{OwnedRef, OwnedRefMut}; use tracing::debug; use super::{CompletedIo, Executor, Item, perform}; +use crate::dist::download::Notifier; use crate::notifications::Notification; #[derive(Copy, Clone, Debug, Enum)] @@ -99,7 +100,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 +110,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 +169,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 +262,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 +283,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 80e8cef3bd..75db01e742 100644 --- a/src/dist/component/package.rs +++ b/src/dist/component/package.rs @@ -16,9 +16,9 @@ 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; @@ -300,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. @@ -635,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 07ccc01810..eac8cd78d2 100644 --- a/src/dist/download.rs +++ b/src/dist/download.rs @@ -24,7 +24,7 @@ const UPDATE_HASH_LEN: usize = 20; pub struct DownloadCfg<'a> { 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, } @@ -34,7 +34,7 @@ impl<'a> DownloadCfg<'a> { DownloadCfg { tmp_cx: &cfg.tmp_cx, download_dir: &cfg.download_dir, - notify_handler: &*cfg.notify_handler, + notifier: Notifier::new(cfg.quiet, cfg.process), process: cfg.process, } } @@ -48,7 +48,7 @@ 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 { debug!("reusing previously downloaded file"); debug!(url = url.as_ref(), "checksum passed"); @@ -77,7 +77,7 @@ impl<'a> DownloadCfg<'a> { &partial_file_path, Some(&mut hasher), true, - &|n| (self.notify_handler)(n), + &self.notifier, self.process, ) .await @@ -126,14 +126,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()) } @@ -174,14 +167,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 { @@ -353,7 +339,7 @@ impl DownloadTracker { } } -fn file_hash(path: &Path, notify_handler: &dyn Fn(Notification<'_>)) -> Result { +fn file_hash(path: &Path, notify_handler: &Notifier) -> Result { let mut hasher = Sha256::new(); let mut downloaded = utils::FileReaderWithProgress::new_file(path, notify_handler)?; use std::io::Read; diff --git a/src/dist/manifestation.rs b/src/dist/manifestation.rs index 98c72ef146..2585350053 100644 --- a/src/dist/manifestation.rs +++ b/src/dist/manifestation.rs @@ -178,12 +178,14 @@ 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), + &self.target_triple, + bin.component.target.as_ref(), + &bin.binary.url, + )); } let semaphore = Arc::new(Semaphore::new(concurrent_downloads)); @@ -280,14 +282,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)?, @@ -452,7 +452,7 @@ impl Manifestation { .unwrap() .replace(DEFAULT_DIST_SERVER, dl_cfg.tmp_cx.dist_server.as_str()); - (dl_cfg.notify_handler)(Notification::DownloadingComponent( + dl_cfg.notifier.handle(Notification::DownloadingComponent( "rust", &self.target_triple, Some(&self.target_triple), @@ -480,11 +480,10 @@ impl Manifestation { } // Install all the components in the installer - let reader = - utils::FileReaderWithProgress::new_file(&installer_file, dl_cfg.notify_handler)?; + let reader = utils::FileReaderWithProgress::new_file(&installer_file, &dl_cfg.notifier)?; let cx = PackageContext { tmp_cx: dl_cfg.tmp_cx, - notify_handler: Some(dl_cfg.notify_handler), + notifier: Some(&dl_cfg.notifier), process: dl_cfg.process, }; @@ -768,7 +767,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 e37516d2c6..39ad76ee34 100644 --- a/src/dist/manifestation/tests.rs +++ b/src/dist/manifestation/tests.rs @@ -16,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, @@ -482,14 +482,21 @@ impl TestContext { let dl_cfg = DownloadCfg { tmp_cx: &self.tmp_cx, download_dir: &self.download_dir, - notify_handler: &|event| println!("{event}"), + notifier: Notifier::new(false, &self.tp.process), process: &self.tp.process, }; // 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)?; diff --git a/src/download/mod.rs b/src/download/mod.rs index 8bdb470099..e13edc6d68 100644 --- a/src/download/mod.rs +++ b/src/download/mod.rs @@ -26,7 +26,9 @@ use tracing::info; use tracing::warn; use url::Url; -use crate::{errors::RustupError, notifications::Notification, process::Process}; +use crate::{ + dist::download::Notifier, errors::RustupError, notifications::Notification, process::Process, +}; #[cfg(test)] mod tests; @@ -35,10 +37,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 +48,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 +87,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 +109,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 +217,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/utils/mod.rs b/src/utils/mod.rs index 0eef74dc39..0ac92c3ff8 100644 --- a/src/utils/mod.rs +++ b/src/utils/mod.rs @@ -14,6 +14,7 @@ use retry::{OperationResult, retry}; use tracing::{debug, info, warn}; use url::Url; +use crate::dist::download::Notifier; use crate::errors::*; use crate::process::Process; @@ -447,16 +448,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 +467,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 +486,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) } From 9702194e853165ad0d8377403d388f1471b3f602 Mon Sep 17 00:00:00 2001 From: Dirkjan Ochtman Date: Sat, 11 Oct 2025 12:52:56 +0200 Subject: [PATCH 17/18] notifications: remove unused Display impl --- src/dist/download.rs | 2 +- src/dist/manifestation.rs | 11 +++-------- src/notifications.rs | 26 +------------------------- 3 files changed, 5 insertions(+), 34 deletions(-) diff --git a/src/dist/download.rs b/src/dist/download.rs index eac8cd78d2..57edd295c5 100644 --- a/src/dist/download.rs +++ b/src/dist/download.rs @@ -254,7 +254,7 @@ impl DownloadTracker { self.download_failed(url); debug!("download failed"); } - Notification::DownloadingComponent(component, _, _, url) => { + Notification::DownloadingComponent(component, url) => { self.create_progress_bar(component.to_owned(), url.to_owned()); } Notification::RetryingDownload(url) => { diff --git a/src/dist/manifestation.rs b/src/dist/manifestation.rs index 2585350053..1e60f58058 100644 --- a/src/dist/manifestation.rs +++ b/src/dist/manifestation.rs @@ -182,8 +182,6 @@ impl Manifestation { .notifier .handle(Notification::DownloadingComponent( &bin.component.short_name(new_manifest), - &self.target_triple, - bin.component.target.as_ref(), &bin.binary.url, )); } @@ -452,12 +450,9 @@ impl Manifestation { .unwrap() .replace(DEFAULT_DIST_SERVER, dl_cfg.tmp_cx.dist_server.as_str()); - dl_cfg.notifier.handle(Notification::DownloadingComponent( - "rust", - &self.target_triple, - Some(&self.target_triple), - &url, - )); + dl_cfg + .notifier + .handle(Notification::DownloadingComponent("rust", &url)); let dl = dl_cfg .download_and_check(&url, update_hash, ".tar.gz") diff --git a/src/notifications.rs b/src/notifications.rs index a2cf911d29..fc164d4ca6 100644 --- a/src/notifications.rs +++ b/src/notifications.rs @@ -1,11 +1,7 @@ -use std::fmt::{self, Display}; - -use crate::dist::TargetTriple; - #[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 TargetTriple, Option<&'a TargetTriple>, &'a str), + 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). @@ -17,23 +13,3 @@ pub(crate) enum Notification<'a> { /// Download has failed. DownloadFailed(&'a str), } - -impl Display for Notification<'_> { - fn fmt(&self, f: &mut fmt::Formatter<'_>) -> std::result::Result<(), fmt::Error> { - use self::Notification::*; - match self { - 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}'"), - 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"), - } - } -} From 552ade941d6ff749ecfb223fb7b40ac86eeb7bce Mon Sep 17 00:00:00 2001 From: Dirkjan Ochtman Date: Sat, 11 Oct 2025 12:54:50 +0200 Subject: [PATCH 18/18] dist: move Notification into dist::download --- src/diskio/threaded.rs | 3 +-- src/dist/download.rs | 21 ++++++++++++++++++--- src/dist/manifestation.rs | 3 +-- src/download/mod.rs | 4 +++- src/lib.rs | 1 - src/notifications.rs | 15 --------------- src/utils/mod.rs | 3 +-- 7 files changed, 24 insertions(+), 26 deletions(-) delete mode 100644 src/notifications.rs diff --git a/src/diskio/threaded.rs b/src/diskio/threaded.rs index afe6fe8e8f..e137be2108 100644 --- a/src/diskio/threaded.rs +++ b/src/diskio/threaded.rs @@ -15,8 +15,7 @@ use sharded_slab::pool::{OwnedRef, OwnedRefMut}; use tracing::debug; use super::{CompletedIo, Executor, Item, perform}; -use crate::dist::download::Notifier; -use crate::notifications::Notification; +use crate::dist::download::{Notification, Notifier}; #[derive(Copy, Clone, Debug, Enum)] pub(crate) enum Bucket { diff --git a/src/dist/download.rs b/src/dist/download.rs index 57edd295c5..e6061c3249 100644 --- a/src/dist/download.rs +++ b/src/dist/download.rs @@ -15,7 +15,6 @@ use crate::config::Cfg; use crate::dist::temp; use crate::download::{download_file, download_file_with_resume}; use crate::errors::*; -use crate::notifications::Notification; use crate::process::Process; use crate::utils; @@ -339,9 +338,25 @@ impl DownloadTracker { } } -fn file_hash(path: &Path, notify_handler: &Notifier) -> Result { +#[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) { diff --git a/src/dist/manifestation.rs b/src/dist/manifestation.rs index 1e60f58058..00c34b0984 100644 --- a/src/dist/manifestation.rs +++ b/src/dist/manifestation.rs @@ -17,14 +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; diff --git a/src/download/mod.rs b/src/download/mod.rs index e13edc6d68..a836927865 100644 --- a/src/download/mod.rs +++ b/src/download/mod.rs @@ -27,7 +27,9 @@ use tracing::warn; use url::Url; use crate::{ - dist::download::Notifier, errors::RustupError, notifications::Notification, process::Process, + dist::download::{Notification, Notifier}, + errors::RustupError, + process::Process, }; #[cfg(test)] 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 fc164d4ca6..0000000000 --- a/src/notifications.rs +++ /dev/null @@ -1,15 +0,0 @@ -#[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), -} diff --git a/src/utils/mod.rs b/src/utils/mod.rs index 0ac92c3ff8..541a872e3b 100644 --- a/src/utils/mod.rs +++ b/src/utils/mod.rs @@ -14,11 +14,10 @@ use retry::{OperationResult, retry}; use tracing::{debug, info, warn}; use url::Url; -use crate::dist::download::Notifier; +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;