From 9e0fb76e27c4cd0351f04b98e8fd67a026f960a2 Mon Sep 17 00:00:00 2001 From: Dirkjan Ochtman Date: Fri, 14 Nov 2025 13:43:59 +0100 Subject: [PATCH 01/20] dist: store specific config bit in Transaction --- src/dist/component/components.rs | 9 ++----- src/dist/component/transaction.rs | 44 +++++++++++++++++-------------- src/dist/download.rs | 9 ++++++- src/dist/manifestation.rs | 25 ++++++++---------- src/dist/manifestation/tests.rs | 7 ++++- src/process.rs | 14 ++++++++++ src/test/dist.rs | 6 ++++- src/utils/mod.rs | 7 ++--- tests/suite/dist_install.rs | 8 ++++-- 9 files changed, 78 insertions(+), 51 deletions(-) diff --git a/src/dist/component/components.rs b/src/dist/component/components.rs index bdc92fefa3..5c2231c61a 100644 --- a/src/dist/component/components.rs +++ b/src/dist/component/components.rs @@ -15,7 +15,6 @@ use crate::dist::component::package::{INSTALLER_VERSION, VERSION_FILE}; use crate::dist::component::transaction::Transaction; use crate::dist::prefix::InstallPrefix; use crate::errors::RustupError; -use crate::process::Process; use crate::utils; const COMPONENTS_FILE: &str = "components"; @@ -255,18 +254,14 @@ impl Component { } Ok(result) } - pub fn uninstall<'a>( - &self, - mut tx: Transaction<'a>, - process: &Process, - ) -> Result> { + pub fn uninstall<'a>(&self, mut tx: Transaction<'a>) -> Result> { // Update components file let path = self.components.rel_components_file(); let abs_path = self.components.prefix.abs_path(&path); let temp = tx.temp().new_file()?; utils::filter_file("components", &abs_path, &temp, |l| l != self.name)?; tx.modify_file(path)?; - utils::rename("components", &temp, &abs_path, process)?; + utils::rename("components", &temp, &abs_path, tx.permit_copy_rename)?; // TODO: If this is the last component remove the components file // and the version file. diff --git a/src/dist/component/transaction.rs b/src/dist/component/transaction.rs index b0f237aba3..733c6d1461 100644 --- a/src/dist/component/transaction.rs +++ b/src/dist/component/transaction.rs @@ -18,7 +18,6 @@ use tracing::{error, info}; use crate::dist::prefix::InstallPrefix; use crate::dist::temp; use crate::errors::RustupError; -use crate::process::Process; use crate::utils; /// A Transaction tracks changes to the file system, allowing them to @@ -39,17 +38,17 @@ pub struct Transaction<'a> { changes: Vec, tmp_cx: &'a temp::Context, committed: bool, - process: &'a Process, + pub(super) permit_copy_rename: bool, } impl<'a> Transaction<'a> { - pub fn new(prefix: InstallPrefix, tmp_cx: &'a temp::Context, process: &'a Process) -> Self { + pub fn new(prefix: InstallPrefix, tmp_cx: &'a temp::Context, permit_copy_rename: bool) -> Self { Transaction { prefix, changes: Vec::new(), tmp_cx, committed: false, - process, + permit_copy_rename, } } @@ -93,7 +92,7 @@ impl<'a> Transaction<'a> { pub fn remove_file(&mut self, component: &str, relpath: PathBuf) -> Result<()> { assert!(relpath.is_relative()); let item = - ChangedItem::remove_file(&self.prefix, component, relpath, self.tmp_cx, self.process)?; + ChangedItem::remove_file(&self.prefix, component, relpath, self.tmp_cx, self.permit_copy_rename)?; self.change(item); Ok(()) } @@ -103,7 +102,7 @@ impl<'a> Transaction<'a> { pub fn remove_dir(&mut self, component: &str, relpath: PathBuf) -> Result<()> { assert!(relpath.is_relative()); let item = - ChangedItem::remove_dir(&self.prefix, component, relpath, self.tmp_cx, self.process)?; + ChangedItem::remove_dir(&self.prefix, component, relpath, self.tmp_cx, self.permit_copy_rename)?; self.change(item); Ok(()) } @@ -142,7 +141,7 @@ impl<'a> Transaction<'a> { src: &Path, ) -> Result<()> { assert!(relpath.is_relative()); - let item = ChangedItem::move_file(&self.prefix, component, relpath, src, self.process)?; + let item = ChangedItem::move_file(&self.prefix, component, relpath, src, self.permit_copy_rename)?; self.change(item); Ok(()) } @@ -150,7 +149,7 @@ impl<'a> Transaction<'a> { /// Recursively move a directory to a relative path of the install prefix. pub(crate) fn move_dir(&mut self, component: &str, relpath: PathBuf, src: &Path) -> Result<()> { assert!(relpath.is_relative()); - let item = ChangedItem::move_dir(&self.prefix, component, relpath, src, self.process)?; + let item = ChangedItem::move_dir(&self.prefix, component, relpath, src, self.permit_copy_rename)?; self.change(item); Ok(()) } @@ -169,7 +168,7 @@ impl Drop for Transaction<'_> { for item in self.changes.iter().rev() { // ok_ntfy!(self.notify_handler, // Notification::NonFatalError, - match item.roll_back(&self.prefix, self.process) { + match item.roll_back(&self.prefix, self.permit_copy_rename) { Ok(()) => {} Err(e) => error!("{e}"), } @@ -192,19 +191,19 @@ enum ChangedItem { } impl ChangedItem { - fn roll_back(&self, prefix: &InstallPrefix, process: &Process) -> Result<()> { + fn roll_back(&self, prefix: &InstallPrefix, permit_copy_rename: bool) -> Result<()> { use self::ChangedItem::*; match self { AddedFile(path) => utils::remove_file("component", &prefix.abs_path(path))?, AddedDir(path) => utils::remove_dir("component", &prefix.abs_path(path))?, RemovedFile(path, tmp) | ModifiedFile(path, Some(tmp)) => { - utils::rename("component", tmp, &prefix.abs_path(path), process)? + utils::rename("component", tmp, &prefix.abs_path(path), permit_copy_rename)? } RemovedDir(path, tmp) => utils::rename( "component", &tmp.join("bk"), &prefix.abs_path(path), - process, + permit_copy_rename, )?, ModifiedFile(path, None) => { let abs_path = prefix.abs_path(path); @@ -260,7 +259,7 @@ impl ChangedItem { component: &str, relpath: PathBuf, tmp_cx: &temp::Context, - process: &Process, + permit_copy_rename: bool, ) -> Result { let abs_path = prefix.abs_path(&relpath); let backup = tmp_cx.new_file()?; @@ -271,7 +270,7 @@ impl ChangedItem { } .into()) } else { - utils::rename("component", &abs_path, &backup, process)?; + utils::rename("component", &abs_path, &backup, permit_copy_rename)?; Ok(ChangedItem::RemovedFile(relpath, backup)) } } @@ -280,7 +279,7 @@ impl ChangedItem { component: &str, relpath: PathBuf, tmp_cx: &temp::Context, - process: &Process, + permit_copy_rename: bool, ) -> Result { let abs_path = prefix.abs_path(&relpath); let backup = tmp_cx.new_directory()?; @@ -291,7 +290,12 @@ impl ChangedItem { } .into()) } else { - utils::rename("component", &abs_path, &backup.join("bk"), process)?; + utils::rename( + "component", + &abs_path, + &backup.join("bk"), + permit_copy_rename, + )?; Ok(ChangedItem::RemovedDir(relpath, backup)) } } @@ -318,10 +322,10 @@ impl ChangedItem { component: &str, relpath: PathBuf, src: &Path, - process: &Process, + permit_copy_rename: bool, ) -> Result { let abs_path = ChangedItem::dest_abs_path(prefix, component, &relpath)?; - utils::rename("component", src, &abs_path, process)?; + utils::rename("component", src, &abs_path, permit_copy_rename)?; Ok(ChangedItem::AddedFile(relpath)) } fn move_dir( @@ -329,10 +333,10 @@ impl ChangedItem { component: &str, relpath: PathBuf, src: &Path, - process: &Process, + permit_copy_rename: bool, ) -> Result { let abs_path = ChangedItem::dest_abs_path(prefix, component, &relpath)?; - utils::rename("component", src, &abs_path, process)?; + utils::rename("component", src, &abs_path, permit_copy_rename)?; Ok(ChangedItem::AddedDir(relpath)) } } diff --git a/src/dist/download.rs b/src/dist/download.rs index 094ef0e058..e9fc404d86 100644 --- a/src/dist/download.rs +++ b/src/dist/download.rs @@ -25,6 +25,7 @@ pub struct DownloadCfg<'a> { pub tmp_cx: &'a temp::Context, pub download_dir: &'a PathBuf, pub(super) tracker: DownloadTracker, + pub(super) permit_copy_rename: bool, pub process: &'a Process, } @@ -35,6 +36,7 @@ impl<'a> DownloadCfg<'a> { tmp_cx: &cfg.tmp_cx, download_dir: &cfg.download_dir, tracker: DownloadTracker::new(!cfg.quiet, cfg.process), + permit_copy_rename: cfg.process.permit_copy_rename(), process: cfg.process, } } @@ -112,7 +114,12 @@ impl<'a> DownloadCfg<'a> { } } else { debug!(url = url.as_ref(), "checksum passed"); - utils::rename("downloaded", &partial_file_path, &target_file, self.process)?; + utils::rename( + "downloaded", + &partial_file_path, + &target_file, + self.permit_copy_rename, + )?; Ok(File { path: target_file }) } } diff --git a/src/dist/manifestation.rs b/src/dist/manifestation.rs index b99775fe8b..80ccbb63cf 100644 --- a/src/dist/manifestation.rs +++ b/src/dist/manifestation.rs @@ -19,7 +19,6 @@ use crate::dist::prefix::InstallPrefix; use crate::dist::temp; use crate::dist::{DEFAULT_DIST_SERVER, Profile, TargetTriple}; use crate::errors::RustupError; -use crate::process::Process; use crate::utils; pub(crate) const DIST_MANIFEST: &str = "multirust-channel-manifest.toml"; @@ -177,11 +176,11 @@ impl Manifestation { .unwrap_or(DEFAULT_MAX_RETRIES); // Begin transaction - let mut tx = Transaction::new(prefix.clone(), tmp_cx, download_cfg.process); + let mut tx = Transaction::new(prefix.clone(), tmp_cx, download_cfg.permit_copy_rename); // If the previous installation was from a v1 manifest we need // to uninstall it first. - tx = self.maybe_handle_v2_upgrade(&config, tx, download_cfg.process)?; + tx = self.maybe_handle_v2_upgrade(&config, tx)?; // Uninstall components for component in &update.components_to_uninstall { @@ -211,7 +210,7 @@ impl Manifestation { } } - tx = self.uninstall_component(component, new_manifest, tx, download_cfg.process)?; + tx = self.uninstall_component(component, new_manifest, tx)?; } info!("downloading component(s)"); @@ -270,11 +269,11 @@ impl Manifestation { &self, manifest: &Manifest, tmp_cx: &temp::Context, - process: &Process, + permit_copy_rename: bool, ) -> Result<()> { let prefix = self.installation.prefix(); - let mut tx = Transaction::new(prefix.clone(), tmp_cx, process); + let mut tx = Transaction::new(prefix.clone(), tmp_cx, permit_copy_rename); // Read configuration and delete it let rel_config_path = prefix.rel_manifest_file(CONFIG_FILE); @@ -287,7 +286,7 @@ impl Manifestation { tx.remove_file("dist config", rel_config_path)?; for component in config.components { - tx = self.uninstall_component(&component, manifest, tx, process)?; + tx = self.uninstall_component(&component, manifest, tx)?; } tx.commit(); @@ -299,7 +298,6 @@ impl Manifestation { component: &Component, manifest: &Manifest, mut tx: Transaction<'a>, - process: &Process, ) -> Result> { // For historical reasons, the rust-installer component // names are not the same as the dist manifest component @@ -308,9 +306,9 @@ impl Manifestation { let name = component.name_in_manifest(); let short_name = component.short_name_in_manifest(); if let Some(c) = self.installation.find(&name)? { - tx = c.uninstall(tx, process)?; + tx = c.uninstall(tx)?; } else if let Some(c) = self.installation.find(short_name)? { - tx = c.uninstall(tx, process)?; + tx = c.uninstall(tx)?; } else { warn!( "component {} not found during uninstall", @@ -398,12 +396,12 @@ impl Manifestation { info!("installing component rust"); // Begin transaction - let mut tx = Transaction::new(prefix, dl_cfg.tmp_cx, dl_cfg.process); + let mut tx = Transaction::new(prefix, dl_cfg.tmp_cx, dl_cfg.permit_copy_rename); // Uninstall components let components = self.installation.list()?; for component in components { - tx = component.uninstall(tx, dl_cfg.process)?; + tx = component.uninstall(tx)?; } // Install all the components in the installer @@ -427,7 +425,6 @@ impl Manifestation { &self, config: &Option, mut tx: Transaction<'a>, - process: &Process, ) -> Result> { let installed_components = self.installation.list()?; let looks_like_v1 = config.is_none() && !installed_components.is_empty(); @@ -437,7 +434,7 @@ impl Manifestation { } for component in installed_components { - tx = component.uninstall(tx, process)?; + tx = component.uninstall(tx)?; } Ok(tx) diff --git a/src/dist/manifestation/tests.rs b/src/dist/manifestation/tests.rs index 376fc4c97e..4fece613f8 100644 --- a/src/dist/manifestation/tests.rs +++ b/src/dist/manifestation/tests.rs @@ -483,6 +483,7 @@ impl TestContext { tmp_cx: &self.tmp_cx, download_dir: &self.download_dir, tracker: DownloadTracker::new(false, &self.tp.process), + permit_copy_rename: self.tp.process.permit_copy_rename(), process: &self.tp.process, }; @@ -524,7 +525,11 @@ impl TestContext { let manifestation = Manifestation::open(self.prefix.clone(), trip)?; let manifest = manifestation.load_manifest()?.unwrap(); - manifestation.uninstall(&manifest, &self.tmp_cx, &self.tp.process)?; + manifestation.uninstall( + &manifest, + &self.tmp_cx, + self.tp.process.permit_copy_rename(), + )?; Ok(()) } diff --git a/src/process.rs b/src/process.rs index 924342d571..b15a43899c 100644 --- a/src/process.rs +++ b/src/process.rs @@ -113,6 +113,20 @@ impl Process { } } + #[cfg(not(target_os = "linux"))] + pub fn permit_copy_rename(&self) -> bool { + false + } + + #[cfg(target_os = "linux")] + pub fn permit_copy_rename(&self) -> bool { + match self { + Process::OsProcess(_) => env::var_os("RUSTUP_PERMIT_COPY_RENAME").is_some(), + #[cfg(feature = "test")] + Process::TestProcess(p) => p.vars.contains_key("RUSTUP_PERMIT_COPY_RENAME"), + } + } + pub(crate) fn var_os(&self, key: &str) -> Option { let value = match self { Process::OsProcess(_) => env::var_os(key)?, diff --git a/src/test/dist.rs b/src/test/dist.rs index 33b42a82ab..e6b13ad431 100644 --- a/src/test/dist.rs +++ b/src/test/dist.rs @@ -62,7 +62,11 @@ impl DistContext { } pub fn transaction(&self) -> Transaction<'_> { - Transaction::new(self.prefix.clone(), &self.cx, &self.tp.process) + Transaction::new( + self.prefix.clone(), + &self.cx, + self.tp.process.permit_copy_rename(), + ) } } diff --git a/src/utils/mod.rs b/src/utils/mod.rs index 0eb30d5387..1fa99388d7 100644 --- a/src/utils/mod.rs +++ b/src/utils/mod.rs @@ -15,7 +15,6 @@ use tracing::{debug, info, warn}; use url::Url; use crate::errors::RustupError; -use crate::process::Process; #[cfg(not(windows))] pub(crate) use crate::utils::raw::find_cmd; @@ -387,7 +386,7 @@ pub fn rename( src: &Path, dest: &Path, #[allow(unused_variables)] // Only used on Linux - process: &Process, + permit_copy_rename: bool, ) -> Result<()> { // https://github.com/rust-lang/rustup/issues/1870 // 21 fib steps from 1 sums to ~28 seconds, hopefully more than enough @@ -410,9 +409,7 @@ pub fn rename( OperationResult::Retry(e) } #[cfg(target_os = "linux")] - _ if process.var_os("RUSTUP_PERMIT_COPY_RENAME").is_some() - && Some(EXDEV) == e.raw_os_error() => - { + _ if permit_copy_rename && Some(EXDEV) == e.raw_os_error() => { match copy_and_delete(name, src, dest) { Ok(()) => OperationResult::Ok(()), Err(_) => OperationResult::Err(e), diff --git a/tests/suite/dist_install.rs b/tests/suite/dist_install.rs index 2908815c97..e22a3691d4 100644 --- a/tests/suite/dist_install.rs +++ b/tests/suite/dist_install.rs @@ -167,9 +167,13 @@ fn uninstall() { tx.commit(); // Now uninstall - let mut tx = Transaction::new(cx.prefix.clone(), &cx.cx, &cx.tp.process); + let mut tx = Transaction::new( + cx.prefix.clone(), + &cx.cx, + cx.tp.process.permit_copy_rename(), + ); for component in components.list().unwrap() { - tx = component.uninstall(tx, &cx.tp.process).unwrap(); + tx = component.uninstall(tx).unwrap(); } tx.commit(); From 9ab1a91fa5a8fd2cb9d6315d170725dc91e769a5 Mon Sep 17 00:00:00 2001 From: Dirkjan Ochtman Date: Fri, 14 Nov 2025 14:23:27 +0100 Subject: [PATCH 02/20] dist: inline single-use tranaction change helpers --- src/dist/component/transaction.rs | 175 +++++++++--------------------- 1 file changed, 54 insertions(+), 121 deletions(-) diff --git a/src/dist/component/transaction.rs b/src/dist/component/transaction.rs index 733c6d1461..fafbb062a3 100644 --- a/src/dist/component/transaction.rs +++ b/src/dist/component/transaction.rs @@ -75,25 +75,36 @@ impl<'a> Transaction<'a> { /// Copy a file to a relative path of the install prefix. pub fn copy_file(&mut self, component: &str, relpath: PathBuf, src: &Path) -> Result<()> { assert!(relpath.is_relative()); - let item = ChangedItem::copy_file(&self.prefix, component, relpath, src)?; - self.change(item); + let abs_path = ChangedItem::dest_abs_path(&self.prefix, component, &relpath)?; + utils::copy_file(src, &abs_path)?; + self.change(ChangedItem::AddedFile(relpath)); Ok(()) } /// Recursively copy a directory to a relative path of the install prefix. pub fn copy_dir(&mut self, component: &str, relpath: PathBuf, src: &Path) -> Result<()> { assert!(relpath.is_relative()); - let item = ChangedItem::copy_dir(&self.prefix, component, relpath, src)?; - self.change(item); + let abs_path = ChangedItem::dest_abs_path(&self.prefix, component, &relpath)?; + utils::copy_dir(src, &abs_path)?; + self.change(ChangedItem::AddedDir(relpath)); Ok(()) } /// Remove a file from a relative path to the install prefix. pub fn remove_file(&mut self, component: &str, relpath: PathBuf) -> Result<()> { assert!(relpath.is_relative()); - let item = - ChangedItem::remove_file(&self.prefix, component, relpath, self.tmp_cx, self.permit_copy_rename)?; - self.change(item); + let abs_path = self.prefix.abs_path(&relpath); + let backup = self.tmp_cx.new_file()?; + if !utils::path_exists(&abs_path) { + return Err(RustupError::ComponentMissingFile { + name: component.to_owned(), + path: relpath, + } + .into()); + } + + utils::rename("component", &abs_path, &backup, self.permit_copy_rename)?; + self.change(ChangedItem::RemovedFile(relpath, backup)); Ok(()) } @@ -101,9 +112,23 @@ impl<'a> Transaction<'a> { /// install prefix. pub fn remove_dir(&mut self, component: &str, relpath: PathBuf) -> Result<()> { assert!(relpath.is_relative()); - let item = - ChangedItem::remove_dir(&self.prefix, component, relpath, self.tmp_cx, self.permit_copy_rename)?; - self.change(item); + let abs_path = self.prefix.abs_path(&relpath); + let backup = self.tmp_cx.new_directory()?; + if !utils::path_exists(&abs_path) { + return Err(RustupError::ComponentMissingDir { + name: component.to_owned(), + path: relpath, + } + .into()); + } + + utils::rename( + "component", + &abs_path, + &backup.join("bk"), + self.permit_copy_rename, + )?; + self.change(ChangedItem::RemovedDir(relpath, backup)); Ok(()) } @@ -128,8 +153,19 @@ impl<'a> Transaction<'a> { /// This is used for arbitrarily manipulating a file. pub fn modify_file(&mut self, relpath: PathBuf) -> Result<()> { assert!(relpath.is_relative()); - let item = ChangedItem::modify_file(&self.prefix, relpath, self.tmp_cx)?; - self.change(item); + let abs_path = self.prefix.abs_path(&relpath); + let backup = if utils::is_file(&abs_path) { + let backup = self.tmp_cx.new_file()?; + utils::copy_file(&abs_path, &backup)?; + Some(backup) + } else { + if let Some(p) = abs_path.parent() { + utils::ensure_dir_exists("component", p)?; + } + None + }; + + self.change(ChangedItem::ModifiedFile(relpath, backup)); Ok(()) } @@ -141,16 +177,18 @@ impl<'a> Transaction<'a> { src: &Path, ) -> Result<()> { assert!(relpath.is_relative()); - let item = ChangedItem::move_file(&self.prefix, component, relpath, src, self.permit_copy_rename)?; - self.change(item); + let abs_path = ChangedItem::dest_abs_path(&self.prefix, component, &relpath)?; + utils::rename("component", src, &abs_path, self.permit_copy_rename)?; + self.change(ChangedItem::AddedFile(relpath)); Ok(()) } /// Recursively move a directory to a relative path of the install prefix. pub(crate) fn move_dir(&mut self, component: &str, relpath: PathBuf, src: &Path) -> Result<()> { assert!(relpath.is_relative()); - let item = ChangedItem::move_dir(&self.prefix, component, relpath, src, self.permit_copy_rename)?; - self.change(item); + let abs_path = ChangedItem::dest_abs_path(&self.prefix, component, &relpath)?; + utils::rename("component", src, &abs_path, self.permit_copy_rename)?; + self.change(ChangedItem::AddedDir(relpath)); Ok(()) } @@ -234,109 +272,4 @@ impl ChangedItem { .with_context(|| format!("error creating file '{}'", abs_path.display()))?; Ok((ChangedItem::AddedFile(relpath), file)) } - fn copy_file( - prefix: &InstallPrefix, - component: &str, - relpath: PathBuf, - src: &Path, - ) -> Result { - let abs_path = ChangedItem::dest_abs_path(prefix, component, &relpath)?; - utils::copy_file(src, &abs_path)?; - Ok(ChangedItem::AddedFile(relpath)) - } - fn copy_dir( - prefix: &InstallPrefix, - component: &str, - relpath: PathBuf, - src: &Path, - ) -> Result { - let abs_path = ChangedItem::dest_abs_path(prefix, component, &relpath)?; - utils::copy_dir(src, &abs_path)?; - Ok(ChangedItem::AddedDir(relpath)) - } - fn remove_file( - prefix: &InstallPrefix, - component: &str, - relpath: PathBuf, - tmp_cx: &temp::Context, - permit_copy_rename: bool, - ) -> Result { - let abs_path = prefix.abs_path(&relpath); - let backup = tmp_cx.new_file()?; - if !utils::path_exists(&abs_path) { - Err(RustupError::ComponentMissingFile { - name: component.to_owned(), - path: relpath, - } - .into()) - } else { - utils::rename("component", &abs_path, &backup, permit_copy_rename)?; - Ok(ChangedItem::RemovedFile(relpath, backup)) - } - } - fn remove_dir( - prefix: &InstallPrefix, - component: &str, - relpath: PathBuf, - tmp_cx: &temp::Context, - permit_copy_rename: bool, - ) -> Result { - let abs_path = prefix.abs_path(&relpath); - let backup = tmp_cx.new_directory()?; - if !utils::path_exists(&abs_path) { - Err(RustupError::ComponentMissingDir { - name: component.to_owned(), - path: relpath, - } - .into()) - } else { - utils::rename( - "component", - &abs_path, - &backup.join("bk"), - permit_copy_rename, - )?; - Ok(ChangedItem::RemovedDir(relpath, backup)) - } - } - fn modify_file( - prefix: &InstallPrefix, - relpath: PathBuf, - tmp_cx: &temp::Context, - ) -> Result { - let abs_path = prefix.abs_path(&relpath); - - if utils::is_file(&abs_path) { - let backup = tmp_cx.new_file()?; - utils::copy_file(&abs_path, &backup)?; - Ok(ChangedItem::ModifiedFile(relpath, Some(backup))) - } else { - if let Some(p) = abs_path.parent() { - utils::ensure_dir_exists("component", p)?; - } - Ok(ChangedItem::ModifiedFile(relpath, None)) - } - } - fn move_file( - prefix: &InstallPrefix, - component: &str, - relpath: PathBuf, - src: &Path, - permit_copy_rename: bool, - ) -> Result { - let abs_path = ChangedItem::dest_abs_path(prefix, component, &relpath)?; - utils::rename("component", src, &abs_path, permit_copy_rename)?; - Ok(ChangedItem::AddedFile(relpath)) - } - fn move_dir( - prefix: &InstallPrefix, - component: &str, - relpath: PathBuf, - src: &Path, - permit_copy_rename: bool, - ) -> Result { - let abs_path = ChangedItem::dest_abs_path(prefix, component, &relpath)?; - utils::rename("component", src, &abs_path, permit_copy_rename)?; - Ok(ChangedItem::AddedDir(relpath)) - } } From b76edbb5600df3695118b86573c6dfdb68dc9495 Mon Sep 17 00:00:00 2001 From: Dirkjan Ochtman Date: Fri, 14 Nov 2025 14:39:33 +0100 Subject: [PATCH 03/20] dist: inline trivial helper function --- src/dist/component/transaction.rs | 23 ++++++++++------------- 1 file changed, 10 insertions(+), 13 deletions(-) diff --git a/src/dist/component/transaction.rs b/src/dist/component/transaction.rs index fafbb062a3..be23d6dbab 100644 --- a/src/dist/component/transaction.rs +++ b/src/dist/component/transaction.rs @@ -58,17 +58,13 @@ impl<'a> Transaction<'a> { self.committed = true; } - fn change(&mut self, item: ChangedItem) { - self.changes.push(item); - } - /// Add a file at a relative path to the install prefix. Returns a /// `File` that may be used to subsequently write the /// contents. pub fn add_file(&mut self, component: &str, relpath: PathBuf) -> Result { assert!(relpath.is_relative()); let (item, file) = ChangedItem::add_file(&self.prefix, component, relpath)?; - self.change(item); + self.changes.push(item); Ok(file) } @@ -77,7 +73,7 @@ impl<'a> Transaction<'a> { assert!(relpath.is_relative()); let abs_path = ChangedItem::dest_abs_path(&self.prefix, component, &relpath)?; utils::copy_file(src, &abs_path)?; - self.change(ChangedItem::AddedFile(relpath)); + self.changes.push(ChangedItem::AddedFile(relpath)); Ok(()) } @@ -86,7 +82,7 @@ impl<'a> Transaction<'a> { assert!(relpath.is_relative()); let abs_path = ChangedItem::dest_abs_path(&self.prefix, component, &relpath)?; utils::copy_dir(src, &abs_path)?; - self.change(ChangedItem::AddedDir(relpath)); + self.changes.push(ChangedItem::AddedDir(relpath)); Ok(()) } @@ -104,7 +100,7 @@ impl<'a> Transaction<'a> { } utils::rename("component", &abs_path, &backup, self.permit_copy_rename)?; - self.change(ChangedItem::RemovedFile(relpath, backup)); + self.changes.push(ChangedItem::RemovedFile(relpath, backup)); Ok(()) } @@ -128,7 +124,7 @@ impl<'a> Transaction<'a> { &backup.join("bk"), self.permit_copy_rename, )?; - self.change(ChangedItem::RemovedDir(relpath, backup)); + self.changes.push(ChangedItem::RemovedDir(relpath, backup)); Ok(()) } @@ -137,13 +133,13 @@ impl<'a> Transaction<'a> { pub fn write_file(&mut self, component: &str, relpath: PathBuf, content: String) -> Result<()> { assert!(relpath.is_relative()); let (item, mut file) = ChangedItem::add_file(&self.prefix, component, relpath.clone())?; - self.change(item); utils::write_str( "component", &mut file, &self.prefix.abs_path(&relpath), &content, )?; + self.changes.push(item); Ok(()) } @@ -165,7 +161,8 @@ impl<'a> Transaction<'a> { None }; - self.change(ChangedItem::ModifiedFile(relpath, backup)); + self.changes + .push(ChangedItem::ModifiedFile(relpath, backup)); Ok(()) } @@ -179,7 +176,7 @@ impl<'a> Transaction<'a> { assert!(relpath.is_relative()); let abs_path = ChangedItem::dest_abs_path(&self.prefix, component, &relpath)?; utils::rename("component", src, &abs_path, self.permit_copy_rename)?; - self.change(ChangedItem::AddedFile(relpath)); + self.changes.push(ChangedItem::AddedFile(relpath)); Ok(()) } @@ -188,7 +185,7 @@ impl<'a> Transaction<'a> { assert!(relpath.is_relative()); let abs_path = ChangedItem::dest_abs_path(&self.prefix, component, &relpath)?; utils::rename("component", src, &abs_path, self.permit_copy_rename)?; - self.change(ChangedItem::AddedDir(relpath)); + self.changes.push(ChangedItem::AddedDir(relpath)); Ok(()) } From f5cf4f33096e0676fe9965ae8cd8e85111ff41f4 Mon Sep 17 00:00:00 2001 From: Dirkjan Ochtman Date: Fri, 14 Nov 2025 14:51:04 +0100 Subject: [PATCH 04/20] dist: inline single-use function --- src/dist/manifestation.rs | 115 +++++++++++++++++--------------------- 1 file changed, 50 insertions(+), 65 deletions(-) diff --git a/src/dist/manifestation.rs b/src/dist/manifestation.rs index 80ccbb63cf..a3eda04f66 100644 --- a/src/dist/manifestation.rs +++ b/src/dist/manifestation.rs @@ -13,7 +13,7 @@ use tracing::{info, warn}; use crate::dist::component::{Components, DirectoryPackage, Transaction}; use crate::dist::config::Config; use crate::dist::download::{DownloadCfg, DownloadStatus, File}; -use crate::dist::manifest::{Component, CompressionKind, HashedBinary, Manifest, TargetedPackage}; +use crate::dist::manifest::{Component, CompressionKind, HashedBinary, Manifest}; use crate::dist::prefix::InstallPrefix; #[cfg(test)] use crate::dist::temp; @@ -488,12 +488,55 @@ impl Update { // Find the final list of components we want to be left with when // we're done: required components, added components, and existing // installed components. - result.build_final_component_list( - &starting_list, - rust_target_package, - new_manifest, - changes, - ); + + // Add requested components + for component in &changes.explicit_add_components { + result.final_component_list.push(component.clone()); + } + + // Add components that are already installed + for existing_component in &starting_list { + let removed = changes.remove_components.contains(existing_component); + + if !removed { + // If there is a rename in the (new) manifest, then we uninstall the component with the + // old name and install a component with the new name + if let Some(renamed_component) = new_manifest.rename_component(existing_component) { + let is_already_included = + result.final_component_list.contains(&renamed_component); + if !is_already_included { + result.final_component_list.push(renamed_component); + } + } else { + let is_already_included = + result.final_component_list.contains(existing_component); + if !is_already_included { + let component_is_present = + rust_target_package.components.contains(existing_component); + + if component_is_present { + result.final_component_list.push(existing_component.clone()); + } else { + // Component not available, check if this is a case of + // where rustup brokenly installed `rust-src` during + // the 1.20.x series + if existing_component.contained_within(&rust_target_package.components) + { + // It is the case, so we need to create a fresh wildcard + // component using the package name and add it to the final + // component list + let wildcarded = existing_component.wildcard(); + if !result.final_component_list.contains(&wildcarded) { + result.final_component_list.push(wildcarded); + } + } else { + result.missing_components.push(existing_component.clone()); + } + } + } + } + } + } // If this is a full upgrade then the list of components to // uninstall is all that are currently installed, and those @@ -541,64 +584,6 @@ impl Update { Ok(result) } - /// Build the list of components we'll have installed at the end - fn build_final_component_list( - &mut self, - starting_list: &[Component], - rust_target_package: &TargetedPackage, - new_manifest: &Manifest, - changes: &Changes, - ) { - // Add requested components - for component in &changes.explicit_add_components { - self.final_component_list.push(component.clone()); - } - - // Add components that are already installed - for existing_component in starting_list { - let removed = changes.remove_components.contains(existing_component); - - if !removed { - // If there is a rename in the (new) manifest, then we uninstall the component with the - // old name and install a component with the new name - if let Some(renamed_component) = new_manifest.rename_component(existing_component) { - let is_already_included = - self.final_component_list.contains(&renamed_component); - if !is_already_included { - self.final_component_list.push(renamed_component); - } - } else { - let is_already_included = - self.final_component_list.contains(existing_component); - if !is_already_included { - let component_is_present = - rust_target_package.components.contains(existing_component); - - if component_is_present { - self.final_component_list.push(existing_component.clone()); - } else { - // Component not available, check if this is a case of - // where rustup brokenly installed `rust-src` during - // the 1.20.x series - if existing_component.contained_within(&rust_target_package.components) - { - // It is the case, so we need to create a fresh wildcard - // component using the package name and add it to the final - // component list - let wildcarded = existing_component.wildcard(); - if !self.final_component_list.contains(&wildcarded) { - self.final_component_list.push(wildcarded); - } - } else { - self.missing_components.push(existing_component.clone()); - } - } - } - } - } - } - } - fn nothing_changes(&self) -> bool { self.components_to_uninstall.is_empty() && self.components_to_install.is_empty() } From d7afc04ece45d880bdea5002d915be93c63b433b Mon Sep 17 00:00:00 2001 From: Dirkjan Ochtman Date: Fri, 14 Nov 2025 14:53:59 +0100 Subject: [PATCH 05/20] dist: linearize for-loop in Update::build_update() --- src/dist/manifestation.rs | 73 +++++++++++++++++++-------------------- 1 file changed, 35 insertions(+), 38 deletions(-) diff --git a/src/dist/manifestation.rs b/src/dist/manifestation.rs index a3eda04f66..6f304e433d 100644 --- a/src/dist/manifestation.rs +++ b/src/dist/manifestation.rs @@ -496,45 +496,42 @@ impl Update { // Add components that are already installed for existing_component in &starting_list { - let removed = changes.remove_components.contains(existing_component); - - if !removed { - // If there is a rename in the (new) manifest, then we uninstall the component with the - // old name and install a component with the new name - if let Some(renamed_component) = new_manifest.rename_component(existing_component) { - let is_already_included = - result.final_component_list.contains(&renamed_component); - if !is_already_included { - result.final_component_list.push(renamed_component); - } - } else { - let is_already_included = - result.final_component_list.contains(existing_component); - if !is_already_included { - let component_is_present = - rust_target_package.components.contains(existing_component); - - if component_is_present { - result.final_component_list.push(existing_component.clone()); - } else { - // Component not available, check if this is a case of - // where rustup brokenly installed `rust-src` during - // the 1.20.x series - if existing_component.contained_within(&rust_target_package.components) - { - // It is the case, so we need to create a fresh wildcard - // component using the package name and add it to the final - // component list - let wildcarded = existing_component.wildcard(); - if !result.final_component_list.contains(&wildcarded) { - result.final_component_list.push(wildcarded); - } - } else { - result.missing_components.push(existing_component.clone()); - } - } - } + if changes.remove_components.contains(existing_component) { + continue; + } + + // If there is a rename in the (new) manifest, then we uninstall the component with the + // old name and install a component with the new name + if let Some(renamed_component) = new_manifest.rename_component(existing_component) { + if !result.final_component_list.contains(&renamed_component) { + result.final_component_list.push(renamed_component); } + continue; + } + + if result.final_component_list.contains(existing_component) { + continue; + } + + if rust_target_package.components.contains(existing_component) { + result.final_component_list.push(existing_component.clone()); + continue; + } + + // Component not available, check if this is a case of + // where rustup brokenly installed `rust-src` during + // the 1.20.x series + if !existing_component.contained_within(&rust_target_package.components) { + result.missing_components.push(existing_component.clone()); + continue; + } + + // It is the case, so we need to create a fresh wildcard + // component using the package name and add it to the final + // component list + let wildcarded = existing_component.wildcard(); + if !result.final_component_list.contains(&wildcarded) { + result.final_component_list.push(wildcarded); } } From 344bfa31c063f344dbcb8190a0dd4e096dc79788 Mon Sep 17 00:00:00 2001 From: Dirkjan Ochtman Date: Fri, 14 Nov 2025 14:54:21 +0100 Subject: [PATCH 06/20] dist: rename Update::build_update() to new() --- src/dist/manifestation.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/dist/manifestation.rs b/src/dist/manifestation.rs index 6f304e433d..f8dc05fa0a 100644 --- a/src/dist/manifestation.rs +++ b/src/dist/manifestation.rs @@ -116,7 +116,7 @@ impl Manifestation { // Create the lists of components needed for installation let config = self.read_config()?; - let mut update = Update::build_update(self, new_manifest, &changes, &config)?; + let mut update = Update::new(self, new_manifest, &changes, &config)?; if update.nothing_changes() { return Ok(UpdateStatus::Unchanged); @@ -452,7 +452,7 @@ struct Update { impl Update { /// Returns components to uninstall, install, and the list of all /// components that will be up to date after the update. - fn build_update( + fn new( manifestation: &Manifestation, new_manifest: &Manifest, changes: &Changes, From 06e388c1b7b85b942d1213f948438814f31d20bc Mon Sep 17 00:00:00 2001 From: Dirkjan Ochtman Date: Fri, 14 Nov 2025 14:55:05 +0100 Subject: [PATCH 07/20] dist: derive trivial initialization for Update --- src/dist/manifestation.rs | 9 ++------- 1 file changed, 2 insertions(+), 7 deletions(-) diff --git a/src/dist/manifestation.rs b/src/dist/manifestation.rs index f8dc05fa0a..b16be490c7 100644 --- a/src/dist/manifestation.rs +++ b/src/dist/manifestation.rs @@ -441,7 +441,7 @@ impl Manifestation { } } -#[derive(Debug)] +#[derive(Debug, Default)] struct Update { components_to_uninstall: Vec, components_to_install: Vec, @@ -478,12 +478,7 @@ impl Update { starting_list.append(&mut profile_components); } - let mut result = Self { - components_to_uninstall: vec![], - components_to_install: vec![], - final_component_list: vec![], - missing_components: vec![], - }; + let mut result = Self::default(); // Find the final list of components we want to be left with when // we're done: required components, added components, and existing From 83a687d0b4c32f93dc5b6dd9c340b7d534431d8c Mon Sep 17 00:00:00 2001 From: Dirkjan Ochtman Date: Fri, 14 Nov 2025 15:16:09 +0100 Subject: [PATCH 08/20] dist: take ownership of manifest in update() --- src/dist/manifestation.rs | 26 +++++++++++++------------- src/dist/manifestation/tests.rs | 2 +- src/dist/mod.rs | 2 +- src/toolchain/distributable.rs | 4 ++-- 4 files changed, 17 insertions(+), 17 deletions(-) diff --git a/src/dist/manifestation.rs b/src/dist/manifestation.rs index b16be490c7..43ec4a375d 100644 --- a/src/dist/manifestation.rs +++ b/src/dist/manifestation.rs @@ -101,7 +101,7 @@ impl Manifestation { /// https://github.com/rust-lang/rustup/issues/988 for the details. pub async fn update( &self, - new_manifest: &Manifest, + new_manifest: Manifest, changes: Changes, force_update: bool, download_cfg: &DownloadCfg<'_>, @@ -116,14 +116,14 @@ impl Manifestation { // Create the lists of components needed for installation let config = self.read_config()?; - let mut update = Update::new(self, new_manifest, &changes, &config)?; + let mut update = Update::new(self, &new_manifest, &changes, &config)?; if update.nothing_changes() { return Ok(UpdateStatus::Unchanged); } // Validate that the requested components are available - if let Err(e) = update.unavailable_components(new_manifest, toolchain_str) { + if let Err(e) = update.unavailable_components(&new_manifest, toolchain_str) { if !force_update { return Err(e); } @@ -134,12 +134,12 @@ impl Manifestation { match &component.target { Some(t) if t != &self.target_triple => warn!( "skipping unavailable component {} for target {}", - component.short_name(new_manifest), + component.short_name(&new_manifest), t ), _ => warn!( "skipping unavailable component {}", - component.short_name(new_manifest) + component.short_name(&new_manifest) ), } } @@ -149,13 +149,13 @@ impl Manifestation { // Download component packages and validate hashes let components = update - .components_urls_and_hashes(new_manifest) + .components_urls_and_hashes(&new_manifest) .map(|res| { res.map(|(component, binary)| ComponentBinary { component, binary, - status: download_cfg.status_for(component.short_name(new_manifest)), - manifest: new_manifest, + status: download_cfg.status_for(component.short_name(&new_manifest)), + manifest: &new_manifest, download_cfg, }) }) @@ -188,29 +188,29 @@ impl Manifestation { (true, Some(t)) if t != &self.target_triple => { info!( "removing previous version of component {} for target {}", - component.short_name(new_manifest), + component.short_name(&new_manifest), t ); } (false, Some(t)) if t != &self.target_triple => { info!( "removing component {} for target {}", - component.short_name(new_manifest), + component.short_name(&new_manifest), t ); } (true, _) => { info!( "removing previous version of component {}", - component.short_name(new_manifest), + component.short_name(&new_manifest), ); } (false, _) => { - info!("removing component {}", component.short_name(new_manifest)); + info!("removing component {}", component.short_name(&new_manifest)); } } - tx = self.uninstall_component(component, new_manifest, tx)?; + tx = self.uninstall_component(component, &new_manifest, tx)?; } info!("downloading component(s)"); diff --git a/src/dist/manifestation/tests.rs b/src/dist/manifestation/tests.rs index 4fece613f8..4c975f6972 100644 --- a/src/dist/manifestation/tests.rs +++ b/src/dist/manifestation/tests.rs @@ -510,7 +510,7 @@ impl TestContext { manifestation .update( - &manifest, + manifest, changes, force, &dl_cfg, diff --git a/src/dist/mod.rs b/src/dist/mod.rs index 14f1c2a19f..21398a6eb7 100644 --- a/src/dist/mod.rs +++ b/src/dist/mod.rs @@ -1202,7 +1202,7 @@ async fn try_update_from_dist_( return match manifestation .update( - &m, + m, changes, force_update, download, diff --git a/src/toolchain/distributable.rs b/src/toolchain/distributable.rs index 10e1231d9d..426bdc43f7 100644 --- a/src/toolchain/distributable.rs +++ b/src/toolchain/distributable.rs @@ -122,7 +122,7 @@ impl<'a> DistributableToolchain<'a> { let download_cfg = DownloadCfg::new(self.toolchain.cfg); manifestation .update( - &manifest, + manifest, changes, false, &download_cfg, @@ -432,7 +432,7 @@ impl<'a> DistributableToolchain<'a> { let download_cfg = DownloadCfg::new(self.toolchain.cfg); manifestation .update( - &manifest, + manifest, changes, false, &download_cfg, From 8db3f2d6133843db1849fb5db56a2da0f8bba62a Mon Sep 17 00:00:00 2001 From: Dirkjan Ochtman Date: Fri, 14 Nov 2025 15:26:21 +0100 Subject: [PATCH 09/20] dist: take ownership of toolchain name in update() --- src/dist/manifestation.rs | 4 ++-- src/dist/manifestation/tests.rs | 2 +- src/dist/mod.rs | 2 +- src/toolchain/distributable.rs | 4 ++-- 4 files changed, 6 insertions(+), 6 deletions(-) diff --git a/src/dist/manifestation.rs b/src/dist/manifestation.rs index 43ec4a375d..8a38fba686 100644 --- a/src/dist/manifestation.rs +++ b/src/dist/manifestation.rs @@ -105,7 +105,7 @@ impl Manifestation { changes: Changes, force_update: bool, download_cfg: &DownloadCfg<'_>, - toolchain_str: &str, + toolchain_str: String, implicit_modify: bool, ) -> Result { // Some vars we're going to need a few times @@ -123,7 +123,7 @@ impl Manifestation { } // Validate that the requested components are available - if let Err(e) = update.unavailable_components(&new_manifest, toolchain_str) { + if let Err(e) = update.unavailable_components(&new_manifest, &toolchain_str) { if !force_update { return Err(e); } diff --git a/src/dist/manifestation/tests.rs b/src/dist/manifestation/tests.rs index 4c975f6972..890f64a7f1 100644 --- a/src/dist/manifestation/tests.rs +++ b/src/dist/manifestation/tests.rs @@ -514,7 +514,7 @@ impl TestContext { changes, force, &dl_cfg, - &self.toolchain.manifest_name(), + self.toolchain.manifest_name(), true, ) .await diff --git a/src/dist/mod.rs b/src/dist/mod.rs index 21398a6eb7..f19dec184e 100644 --- a/src/dist/mod.rs +++ b/src/dist/mod.rs @@ -1206,7 +1206,7 @@ async fn try_update_from_dist_( changes, force_update, download, - &toolchain.manifest_name(), + toolchain.manifest_name(), true, ) .await diff --git a/src/toolchain/distributable.rs b/src/toolchain/distributable.rs index 426bdc43f7..cbcb2e2238 100644 --- a/src/toolchain/distributable.rs +++ b/src/toolchain/distributable.rs @@ -126,7 +126,7 @@ impl<'a> DistributableToolchain<'a> { changes, false, &download_cfg, - &self.desc.manifest_name(), + self.desc.manifest_name(), false, ) .await?; @@ -436,7 +436,7 @@ impl<'a> DistributableToolchain<'a> { changes, false, &download_cfg, - &self.desc.manifest_name(), + self.desc.manifest_name(), false, ) .await?; From 3c15874f8879ff24cb85f5d81bdd6eafdf40b27f Mon Sep 17 00:00:00 2001 From: Dirkjan Ochtman Date: Fri, 14 Nov 2025 15:35:20 +0100 Subject: [PATCH 10/20] dist: take ownership of existing Components --- src/dist/manifestation.rs | 70 +++++++++++++++++---------------------- 1 file changed, 30 insertions(+), 40 deletions(-) diff --git a/src/dist/manifestation.rs b/src/dist/manifestation.rs index 8a38fba686..f16933067f 100644 --- a/src/dist/manifestation.rs +++ b/src/dist/manifestation.rs @@ -149,15 +149,32 @@ impl Manifestation { // Download component packages and validate hashes let components = update - .components_urls_and_hashes(&new_manifest) - .map(|res| { - res.map(|(component, binary)| ComponentBinary { - component, - binary, - status: download_cfg.status_for(component.short_name(&new_manifest)), - manifest: &new_manifest, - download_cfg, - }) + .components_to_install + .into_iter() + .filter_map(|component| { + let package = match new_manifest.get_package(component.short_name_in_manifest()) { + Ok(p) => p, + Err(e) => return Some(Err(e)), + }; + + let target_package = match package.get_target(component.target.as_ref()) { + Ok(tp) => tp, + Err(e) => return Some(Err(e)), + }; + + match target_package.bins.is_empty() { + // This package is not available, no files to download. + true => None, + // We prefer the first format in the list, since the parsing of the + // manifest leaves us with the files/hash pairs in preference order. + false => Some(Ok(ComponentBinary { + status: download_cfg.status_for(component.short_name(&new_manifest)), + component, + binary: &target_package.bins[0], + manifest: &new_manifest, + download_cfg, + })), + } }) .collect::>>()?; @@ -612,36 +629,10 @@ impl Update { self.components_to_install.retain(|c| !to_drop.contains(c)); self.final_component_list.retain(|c| !to_drop.contains(c)); } - - /// Map components to urls and hashes - fn components_urls_and_hashes<'a>( - &'a self, - new_manifest: &'a Manifest, - ) -> impl Iterator> + 'a { - self.components_to_install.iter().filter_map(|component| { - let package = match new_manifest.get_package(component.short_name_in_manifest()) { - Ok(p) => p, - Err(e) => return Some(Err(e)), - }; - - let target_package = match package.get_target(component.target.as_ref()) { - Ok(tp) => tp, - Err(e) => return Some(Err(e)), - }; - - match target_package.bins.is_empty() { - // This package is not available, no files to download. - true => None, - // We prefer the first format in the list, since the parsing of the - // manifest leaves us with the files/hash pairs in preference order. - false => Some(Ok((component, &target_package.bins[0]))), - } - }) - } } struct ComponentBinary<'a> { - component: &'a Component, + component: Component, binary: &'a HashedBinary, status: DownloadStatus, manifest: &'a Manifest, @@ -689,10 +680,9 @@ impl<'a> ComponentBinary<'a> { // names are not the same as the dist manifest component // names. Some are just the component name some are the // component name plus the target triple. - let component = self.component; - let pkg_name = component.name_in_manifest(); - let short_pkg_name = component.short_name_in_manifest(); - let short_name = component.short_name(self.manifest); + let pkg_name = self.component.name_in_manifest(); + let short_pkg_name = self.component.short_name_in_manifest(); + let short_name = self.component.short_name(self.manifest); self.status.installing(); From 229d3a4436321845475b92fb357c4cf53d5c918b Mon Sep 17 00:00:00 2001 From: Dirkjan Ochtman Date: Fri, 14 Nov 2025 15:39:18 +0100 Subject: [PATCH 11/20] dist: transfer ownership of component values --- src/dist/manifestation.rs | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/src/dist/manifestation.rs b/src/dist/manifestation.rs index f16933067f..50d458adbb 100644 --- a/src/dist/manifestation.rs +++ b/src/dist/manifestation.rs @@ -200,7 +200,7 @@ impl Manifestation { tx = self.maybe_handle_v2_upgrade(&config, tx)?; // Uninstall components - for component in &update.components_to_uninstall { + for component in update.components_to_uninstall { match (implicit_modify, &component.target) { (true, Some(t)) if t != &self.target_triple => { info!( @@ -232,7 +232,7 @@ impl Manifestation { info!("downloading component(s)"); let mut downloads = FuturesUnordered::new(); - let mut component_iter = components.iter(); + let mut component_iter = components.into_iter(); let mut cleanup_downloads = vec![]; loop { if downloads.is_empty() && component_iter.len() == 0 { @@ -303,7 +303,7 @@ impl Manifestation { tx.remove_file("dist config", rel_config_path)?; for component in config.components { - tx = self.uninstall_component(&component, manifest, tx)?; + tx = self.uninstall_component(component, manifest, tx)?; } tx.commit(); @@ -312,7 +312,7 @@ impl Manifestation { fn uninstall_component<'a>( &self, - component: &Component, + component: Component, manifest: &Manifest, mut tx: Transaction<'a>, ) -> Result> { @@ -640,7 +640,7 @@ struct ComponentBinary<'a> { } impl<'a> ComponentBinary<'a> { - async fn download(&self, max_retries: usize) -> Result<(&Self, File)> { + async fn download(self, max_retries: usize) -> Result<(Self, File)> { use tokio_retry::{RetryIf, strategy::FixedInterval}; let url = self.download_cfg.url(&self.binary.url)?; @@ -671,7 +671,7 @@ impl<'a> ComponentBinary<'a> { } fn install<'t>( - &self, + self, installer_file: File, tx: Transaction<'t>, manifestation: &Manifestation, From 1646c489cd8be422402de05cb90cf566a73c846b Mon Sep 17 00:00:00 2001 From: Dirkjan Ochtman Date: Fri, 14 Nov 2025 15:43:03 +0100 Subject: [PATCH 12/20] diskio: clarify dependency on I/O thread count --- src/diskio/mod.rs | 10 ++++------ src/diskio/test.rs | 6 ++++-- src/dist/component/package.rs | 2 +- 3 files changed, 9 insertions(+), 9 deletions(-) diff --git a/src/diskio/mod.rs b/src/diskio/mod.rs index 3d00241441..e51f236cfc 100644 --- a/src/diskio/mod.rs +++ b/src/diskio/mod.rs @@ -66,8 +66,6 @@ use std::{fmt::Debug, fs::OpenOptions}; use anyhow::Result; -use crate::process::Process; - /// Carries the implementation specific data for complete file transfers into the executor. #[derive(Debug)] pub(crate) enum FileBuffer { @@ -443,11 +441,11 @@ pub(crate) fn create_dir>(path: P) -> io::Result<()> { /// Get the executor for disk IO. pub(crate) fn get_executor<'a>( ram_budget: usize, - process: &Process, -) -> Result> { + io_thread_count: usize, +) -> Box { // If this gets lots of use, consider exposing via the config file. - Ok(match process.io_thread_count()? { + match io_thread_count { 0 | 1 => Box::new(immediate::ImmediateUnpacker::new()), n => Box::new(threaded::Threaded::new(n, ram_budget)), - }) + } } diff --git a/src/diskio/test.rs b/src/diskio/test.rs index 738d39e27b..0a893162f9 100644 --- a/src/diskio/test.rs +++ b/src/diskio/test.rs @@ -24,7 +24,8 @@ fn test_incremental_file(io_threads: &str) -> Result<()> { let mut written = 0; let mut file_finished = false; - let mut io_executor: Box = get_executor(32 * 1024 * 1024, &tp.process)?; + let mut io_executor: Box = + get_executor(32 * 1024 * 1024, tp.process.io_thread_count()?); let (item, mut sender) = Item::write_file_segmented( work_dir.path().join("scratch"), 0o666, @@ -90,7 +91,8 @@ fn test_complete_file(io_threads: &str) -> Result<()> { vars.insert("RUSTUP_IO_THREADS".to_string(), io_threads.to_string()); let tp = TestProcess::with_vars(vars); - let mut io_executor: Box = get_executor(32 * 1024 * 1024, &tp.process)?; + let mut io_executor: Box = + get_executor(32 * 1024 * 1024, tp.process.io_thread_count()?); let mut chunk = io_executor.get_buffer(10); chunk.extend(b"0123456789"); assert_eq!(chunk.len(), 10); diff --git a/src/dist/component/package.rs b/src/dist/component/package.rs index 9ca699fb8a..eb2a6d01d8 100644 --- a/src/dist/component/package.rs +++ b/src/dist/component/package.rs @@ -285,7 +285,7 @@ fn unpack_without_first_dir( } }; let unpack_ram = unpack_ram(IO_CHUNK_SIZE, effective_max_ram, dl_cfg); - let mut io_executor: Box = get_executor(unpack_ram, dl_cfg.process)?; + let mut io_executor = get_executor(unpack_ram, dl_cfg.process.io_thread_count()?); let mut directories: HashMap = HashMap::new(); // Path is presumed to exist. Call it a precondition. From 6c55cd1a1ff272365ff6fc19fb459e06a00d39fe Mon Sep 17 00:00:00 2001 From: Dirkjan Ochtman Date: Fri, 14 Nov 2025 15:48:11 +0100 Subject: [PATCH 13/20] dist: clarify dependency on unpack RAM budget --- src/dist/component/package.rs | 15 +++++---------- src/process.rs | 7 +++++++ 2 files changed, 12 insertions(+), 10 deletions(-) diff --git a/src/dist/component/package.rs b/src/dist/component/package.rs index eb2a6d01d8..0ca40795ca 100644 --- a/src/dist/component/package.rs +++ b/src/dist/component/package.rs @@ -4,10 +4,10 @@ use std::collections::{HashMap, HashSet}; use std::io::{self, ErrorKind as IOErrorKind, Read}; -use std::mem; use std::ops::Deref; use std::path::{Path, PathBuf}; use std::sync::OnceLock; +use std::{env, mem}; use anyhow::{Context, Result, anyhow, bail}; use tar::EntryType; @@ -149,7 +149,7 @@ fn unpack_ram( io_chunk_size: usize, effective_max_ram: Option, dl_cfg: &DownloadCfg<'_>, -) -> usize { +) -> Result { const RAM_ALLOWANCE_FOR_RUSTUP_AND_BUFFERS: usize = 200 * 1024 * 1024; let minimum_ram = io_chunk_size * 2; let default_max_unpack_ram = if let Some(effective_max_ram) = effective_max_ram { @@ -162,12 +162,7 @@ fn unpack_ram( // Rustup does not know how much RAM the machine has: use the minimum minimum_ram }; - let unpack_ram = match dl_cfg - .process - .var("RUSTUP_UNPACK_RAM") - .ok() - .and_then(|budget_str| budget_str.parse::().ok()) - { + let unpack_ram = match dl_cfg.process.unpack_ram()? { Some(budget) => { if budget < minimum_ram { warn!( @@ -196,7 +191,7 @@ fn unpack_ram( if minimum_ram > unpack_ram { panic!("RUSTUP_UNPACK_RAM must be larger than {minimum_ram}"); } else { - unpack_ram + Ok(unpack_ram) } } @@ -284,7 +279,7 @@ fn unpack_without_first_dir( None } }; - let unpack_ram = unpack_ram(IO_CHUNK_SIZE, effective_max_ram, dl_cfg); + let unpack_ram = unpack_ram(IO_CHUNK_SIZE, effective_max_ram, dl_cfg)?; let mut io_executor = get_executor(unpack_ram, dl_cfg.process.io_thread_count()?); let mut directories: HashMap = HashMap::new(); diff --git a/src/process.rs b/src/process.rs index b15a43899c..4960351979 100644 --- a/src/process.rs +++ b/src/process.rs @@ -89,6 +89,13 @@ impl Process { }) } + pub(crate) fn unpack_ram(&self) -> Result, env::VarError> { + Ok(match self.var_opt("RUSTUP_UNPACK_RAM")? { + Some(budget) => usize::from_str(&budget).ok(), + None => None, + }) + } + pub fn var_opt(&self, key: &str) -> Result, env::VarError> { match self.var(key) { Ok(val) => Ok(Some(val)), From 5ae79ae1e3e286f0fea47a16f7fd2ee8a4ccb655 Mon Sep 17 00:00:00 2001 From: Dirkjan Ochtman Date: Fri, 14 Nov 2025 15:53:12 +0100 Subject: [PATCH 14/20] dist: use logging for missing parent warnings --- src/dist/component/package.rs | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/src/dist/component/package.rs b/src/dist/component/package.rs index 0ca40795ca..ad9ded7b0e 100644 --- a/src/dist/component/package.rs +++ b/src/dist/component/package.rs @@ -439,13 +439,11 @@ fn unpack_without_first_dir( None => { // Tar has item before containing directory // Complain about this so we can see if these exist. - use std::io::Write as _; - writeln!( - dl_cfg.process.stderr().lock(), - "Unexpected: missing parent '{}' for '{}'", + warn!( + "unexpected: missing parent '{}' for '{}'", parent.display(), entry.path()?.display() - )?; + ); directories.insert(parent.to_owned(), DirStatus::Pending(vec![item])); item = Item::make_dir(parent.to_owned(), 0o755); // Check the parent's parent From 8681e5ce98eff5beeb50e9a9edbd7df92dd9cb12 Mon Sep 17 00:00:00 2001 From: Dirkjan Ochtman Date: Fri, 14 Nov 2025 16:08:48 +0100 Subject: [PATCH 15/20] dist: hoist environment variable extraction --- src/dist/component/package.rs | 27 +++++++++++++++++---------- 1 file changed, 17 insertions(+), 10 deletions(-) diff --git a/src/dist/component/package.rs b/src/dist/component/package.rs index ad9ded7b0e..dd2331661c 100644 --- a/src/dist/component/package.rs +++ b/src/dist/component/package.rs @@ -4,10 +4,10 @@ use std::collections::{HashMap, HashSet}; use std::io::{self, ErrorKind as IOErrorKind, Read}; +use std::mem; use std::ops::Deref; use std::path::{Path, PathBuf}; use std::sync::OnceLock; -use std::{env, mem}; use anyhow::{Context, Result, anyhow, bail}; use tar::EntryType; @@ -52,11 +52,17 @@ impl DirectoryPackage { fn from_tar(stream: impl Read, dl_cfg: &DownloadCfg<'_>) -> Result { let temp_dir = dl_cfg.tmp_cx.new_directory()?; let mut archive = tar::Archive::new(stream); + // The rust-installer packages unpack to a directory called // $pkgname-$version-$target. Skip that directory when // unpacking. - unpack_without_first_dir(&mut archive, &temp_dir, dl_cfg) - .context("failed to extract package")?; + unpack_without_first_dir( + &mut archive, + &temp_dir, + dl_cfg.process.unpack_ram()?, + dl_cfg.process.io_thread_count()?, + ) + .context("failed to extract package")?; Self::new(temp_dir, false) } @@ -148,8 +154,8 @@ impl> DirectoryPackage

{ fn unpack_ram( io_chunk_size: usize, effective_max_ram: Option, - dl_cfg: &DownloadCfg<'_>, -) -> Result { + budget: Option, +) -> usize { const RAM_ALLOWANCE_FOR_RUSTUP_AND_BUFFERS: usize = 200 * 1024 * 1024; let minimum_ram = io_chunk_size * 2; let default_max_unpack_ram = if let Some(effective_max_ram) = effective_max_ram { @@ -162,7 +168,7 @@ fn unpack_ram( // Rustup does not know how much RAM the machine has: use the minimum minimum_ram }; - let unpack_ram = match dl_cfg.process.unpack_ram()? { + let unpack_ram = match budget { Some(budget) => { if budget < minimum_ram { warn!( @@ -191,7 +197,7 @@ fn unpack_ram( if minimum_ram > unpack_ram { panic!("RUSTUP_UNPACK_RAM must be larger than {minimum_ram}"); } else { - Ok(unpack_ram) + unpack_ram } } @@ -269,7 +275,8 @@ enum DirStatus { fn unpack_without_first_dir( archive: &mut tar::Archive, path: &Path, - dl_cfg: &DownloadCfg<'_>, + unpack_ram_budget: Option, + io_thread_count: usize, ) -> Result<()> { let entries = archive.entries()?; let effective_max_ram = match effective_limits::memory_limit() { @@ -279,8 +286,8 @@ fn unpack_without_first_dir( None } }; - let unpack_ram = unpack_ram(IO_CHUNK_SIZE, effective_max_ram, dl_cfg)?; - let mut io_executor = get_executor(unpack_ram, dl_cfg.process.io_thread_count()?); + let unpack_ram = unpack_ram(IO_CHUNK_SIZE, effective_max_ram, unpack_ram_budget); + let mut io_executor = get_executor(unpack_ram, io_thread_count); let mut directories: HashMap = HashMap::new(); // Path is presumed to exist. Call it a precondition. From ae8866b0ff674c236d7e5c0a90b64647fe9fe06e Mon Sep 17 00:00:00 2001 From: Dirkjan Ochtman Date: Fri, 14 Nov 2025 16:19:29 +0100 Subject: [PATCH 16/20] dist: inline effective RAM limit calculation --- src/dist/component/package.rs | 33 +++++++++++++-------------------- 1 file changed, 13 insertions(+), 20 deletions(-) diff --git a/src/dist/component/package.rs b/src/dist/component/package.rs index dd2331661c..ce7ac9627f 100644 --- a/src/dist/component/package.rs +++ b/src/dist/component/package.rs @@ -151,23 +151,23 @@ impl> DirectoryPackage

{ } // Probably this should live in diskio but ¯\_(ツ)_/¯ -fn unpack_ram( - io_chunk_size: usize, - effective_max_ram: Option, - budget: Option, -) -> usize { +fn unpack_ram(io_chunk_size: usize, budget: Option) -> usize { const RAM_ALLOWANCE_FOR_RUSTUP_AND_BUFFERS: usize = 200 * 1024 * 1024; let minimum_ram = io_chunk_size * 2; - let default_max_unpack_ram = if let Some(effective_max_ram) = effective_max_ram { - if effective_max_ram > minimum_ram + RAM_ALLOWANCE_FOR_RUSTUP_AND_BUFFERS { - effective_max_ram - RAM_ALLOWANCE_FOR_RUSTUP_AND_BUFFERS - } else { + + let default_max_unpack_ram = match effective_limits::memory_limit() { + Ok(effective) + if effective as usize > minimum_ram + RAM_ALLOWANCE_FOR_RUSTUP_AND_BUFFERS => + { + effective as usize - RAM_ALLOWANCE_FOR_RUSTUP_AND_BUFFERS + } + Ok(_) => minimum_ram, + Err(error) => { + error!("can't determine memory limit: {error}"); minimum_ram } - } else { - // Rustup does not know how much RAM the machine has: use the minimum - minimum_ram }; + let unpack_ram = match budget { Some(budget) => { if budget < minimum_ram { @@ -279,14 +279,7 @@ fn unpack_without_first_dir( io_thread_count: usize, ) -> Result<()> { let entries = archive.entries()?; - let effective_max_ram = match effective_limits::memory_limit() { - Ok(ram) => Some(ram as usize), - Err(error) => { - error!("can't determine memory limit: {error}"); - None - } - }; - let unpack_ram = unpack_ram(IO_CHUNK_SIZE, effective_max_ram, unpack_ram_budget); + let unpack_ram = unpack_ram(IO_CHUNK_SIZE, unpack_ram_budget); let mut io_executor = get_executor(unpack_ram, io_thread_count); let mut directories: HashMap = HashMap::new(); From a227bc8fde5513542db455539718c11d6c746f2d Mon Sep 17 00:00:00 2001 From: Dirkjan Ochtman Date: Fri, 14 Nov 2025 16:21:24 +0100 Subject: [PATCH 17/20] dist: hoist Executor creation up --- src/dist/component/package.rs | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) diff --git a/src/dist/component/package.rs b/src/dist/component/package.rs index ce7ac9627f..477fa726ee 100644 --- a/src/dist/component/package.rs +++ b/src/dist/component/package.rs @@ -59,8 +59,10 @@ impl DirectoryPackage { unpack_without_first_dir( &mut archive, &temp_dir, - dl_cfg.process.unpack_ram()?, - dl_cfg.process.io_thread_count()?, + get_executor( + unpack_ram(IO_CHUNK_SIZE, dl_cfg.process.unpack_ram()?), + dl_cfg.process.io_thread_count()?, + ), ) .context("failed to extract package")?; @@ -275,13 +277,9 @@ enum DirStatus { fn unpack_without_first_dir( archive: &mut tar::Archive, path: &Path, - unpack_ram_budget: Option, - io_thread_count: usize, + mut io_executor: Box, ) -> Result<()> { let entries = archive.entries()?; - let unpack_ram = unpack_ram(IO_CHUNK_SIZE, unpack_ram_budget); - let mut io_executor = get_executor(unpack_ram, io_thread_count); - let mut directories: HashMap = HashMap::new(); // Path is presumed to exist. Call it a precondition. directories.insert(path.to_owned(), DirStatus::Exists); From ccaae1488469eb5500cb136c309887e16485b0c7 Mon Sep 17 00:00:00 2001 From: Dirkjan Ochtman Date: Fri, 14 Nov 2025 16:26:07 +0100 Subject: [PATCH 18/20] Move unpack_ram() from dist to diskio --- src/diskio/mod.rs | 68 +++++++++++++++++++++++++++++++---- src/dist/component/package.rs | 61 +++---------------------------- 2 files changed, 66 insertions(+), 63 deletions(-) diff --git a/src/diskio/mod.rs b/src/diskio/mod.rs index e51f236cfc..bfb09ebb44 100644 --- a/src/diskio/mod.rs +++ b/src/diskio/mod.rs @@ -51,20 +51,24 @@ // loss or errors in this model. // f) data gathering: record (name, bytes, start, duration) // write to disk afterwards as a csv file? -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}; use std::path::{Path, PathBuf}; +use std::sync::OnceLock; use std::sync::mpsc::Receiver; use std::time::{Duration, Instant}; use std::{fmt::Debug, fs::OpenOptions}; use anyhow::Result; +use tracing::{error, trace, warn}; + +use crate::utils::units::Size; + +pub(crate) mod immediate; +#[cfg(test)] +mod test; +pub(crate) mod threaded; +use threaded::PoolReference; /// Carries the implementation specific data for complete file transfers into the executor. #[derive(Debug)] @@ -449,3 +453,55 @@ pub(crate) fn get_executor<'a>( n => Box::new(threaded::Threaded::new(n, ram_budget)), } } + +pub(crate) fn unpack_ram(io_chunk_size: usize, budget: Option) -> usize { + const RAM_ALLOWANCE_FOR_RUSTUP_AND_BUFFERS: usize = 200 * 1024 * 1024; + let minimum_ram = io_chunk_size * 2; + + let default_max_unpack_ram = match effective_limits::memory_limit() { + Ok(effective) + if effective as usize > minimum_ram + RAM_ALLOWANCE_FOR_RUSTUP_AND_BUFFERS => + { + effective as usize - RAM_ALLOWANCE_FOR_RUSTUP_AND_BUFFERS + } + Ok(_) => minimum_ram, + Err(error) => { + error!("can't determine memory limit: {error}"); + minimum_ram + } + }; + + let unpack_ram = match budget { + Some(budget) => { + if budget < minimum_ram { + warn!( + "Ignoring RUSTUP_UNPACK_RAM ({}) less than minimum of {}.", + budget, minimum_ram + ); + minimum_ram + } else if budget > default_max_unpack_ram { + warn!( + "Ignoring RUSTUP_UNPACK_RAM ({}) greater than detected available RAM of {}.", + budget, default_max_unpack_ram + ); + default_max_unpack_ram + } else { + budget + } + } + None => { + if RAM_NOTICE_SHOWN.set(()).is_ok() { + trace!(size = %Size::new(default_max_unpack_ram), "unpacking components in memory"); + } + default_max_unpack_ram + } + }; + + if minimum_ram > unpack_ram { + panic!("RUSTUP_UNPACK_RAM must be larger than {minimum_ram}"); + } else { + unpack_ram + } +} + +static RAM_NOTICE_SHOWN: OnceLock<()> = OnceLock::new(); diff --git a/src/dist/component/package.rs b/src/dist/component/package.rs index 477fa726ee..45bb17fc66 100644 --- a/src/dist/component/package.rs +++ b/src/dist/component/package.rs @@ -7,13 +7,14 @@ use std::io::{self, ErrorKind as IOErrorKind, Read}; use std::mem; use std::ops::Deref; use std::path::{Path, PathBuf}; -use std::sync::OnceLock; use anyhow::{Context, Result, anyhow, bail}; use tar::EntryType; -use tracing::{error, trace, warn}; +use tracing::warn; -use crate::diskio::{CompletedIo, Executor, FileBuffer, IO_CHUNK_SIZE, Item, Kind, get_executor}; +use crate::diskio::{ + CompletedIo, Executor, FileBuffer, IO_CHUNK_SIZE, Item, Kind, get_executor, unpack_ram, +}; use crate::dist::component::components::{ComponentPart, ComponentPartKind, Components}; use crate::dist::component::transaction::Transaction; use crate::dist::download::DownloadCfg; @@ -21,7 +22,6 @@ use crate::dist::manifest::CompressionKind; use crate::dist::temp; use crate::errors::RustupError; use crate::utils; -use crate::utils::units::Size; /// The current metadata revision used by rust-installer pub(crate) const INSTALLER_VERSION: &str = "3"; @@ -152,59 +152,6 @@ impl> DirectoryPackage

{ } } -// Probably this should live in diskio but ¯\_(ツ)_/¯ -fn unpack_ram(io_chunk_size: usize, budget: Option) -> usize { - const RAM_ALLOWANCE_FOR_RUSTUP_AND_BUFFERS: usize = 200 * 1024 * 1024; - let minimum_ram = io_chunk_size * 2; - - let default_max_unpack_ram = match effective_limits::memory_limit() { - Ok(effective) - if effective as usize > minimum_ram + RAM_ALLOWANCE_FOR_RUSTUP_AND_BUFFERS => - { - effective as usize - RAM_ALLOWANCE_FOR_RUSTUP_AND_BUFFERS - } - Ok(_) => minimum_ram, - Err(error) => { - error!("can't determine memory limit: {error}"); - minimum_ram - } - }; - - let unpack_ram = match budget { - Some(budget) => { - if budget < minimum_ram { - warn!( - "Ignoring RUSTUP_UNPACK_RAM ({}) less than minimum of {}.", - budget, minimum_ram - ); - minimum_ram - } else if budget > default_max_unpack_ram { - warn!( - "Ignoring RUSTUP_UNPACK_RAM ({}) greater than detected available RAM of {}.", - budget, default_max_unpack_ram - ); - default_max_unpack_ram - } else { - budget - } - } - None => { - if RAM_NOTICE_SHOWN.set(()).is_ok() { - trace!(size = %Size::new(default_max_unpack_ram), "unpacking components in memory"); - } - default_max_unpack_ram - } - }; - - if minimum_ram > unpack_ram { - panic!("RUSTUP_UNPACK_RAM must be larger than {minimum_ram}"); - } else { - 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<()> { From d9ea4527f2e701127690cebe8e330eefe0ebfec1 Mon Sep 17 00:00:00 2001 From: Dirkjan Ochtman Date: Fri, 14 Nov 2025 16:28:13 +0100 Subject: [PATCH 19/20] dist: hoist creation of io_executor some more --- src/dist/component/package.rs | 40 +++++++++++++++++------------------ src/dist/manifestation.rs | 16 ++++++++++++-- 2 files changed, 34 insertions(+), 22 deletions(-) diff --git a/src/dist/component/package.rs b/src/dist/component/package.rs index 45bb17fc66..0cc5855fe6 100644 --- a/src/dist/component/package.rs +++ b/src/dist/component/package.rs @@ -12,12 +12,9 @@ use anyhow::{Context, Result, anyhow, bail}; use tar::EntryType; use tracing::warn; -use crate::diskio::{ - CompletedIo, Executor, FileBuffer, IO_CHUNK_SIZE, Item, Kind, get_executor, unpack_ram, -}; +use crate::diskio::{CompletedIo, Executor, FileBuffer, IO_CHUNK_SIZE, Item, Kind}; use crate::dist::component::components::{ComponentPart, ComponentPartKind, Components}; use crate::dist::component::transaction::Transaction; -use crate::dist::download::DownloadCfg; use crate::dist::manifest::CompressionKind; use crate::dist::temp; use crate::errors::RustupError; @@ -38,33 +35,36 @@ impl DirectoryPackage { pub(crate) fn compressed( stream: R, kind: CompressionKind, - dl_cfg: &DownloadCfg<'_>, + temp_dir: temp::Dir, + io_executor: Box, ) -> Result { match kind { - CompressionKind::GZip => Self::from_tar(flate2::read::GzDecoder::new(stream), dl_cfg), - CompressionKind::ZStd => { - Self::from_tar(zstd::stream::read::Decoder::new(stream)?, dl_cfg) + CompressionKind::GZip => { + Self::from_tar(flate2::read::GzDecoder::new(stream), temp_dir, io_executor) + } + CompressionKind::ZStd => Self::from_tar( + zstd::stream::read::Decoder::new(stream)?, + temp_dir, + io_executor, + ), + CompressionKind::XZ => { + Self::from_tar(xz2::read::XzDecoder::new(stream), temp_dir, io_executor) } - CompressionKind::XZ => Self::from_tar(xz2::read::XzDecoder::new(stream), dl_cfg), } } - fn from_tar(stream: impl Read, dl_cfg: &DownloadCfg<'_>) -> Result { - let temp_dir = dl_cfg.tmp_cx.new_directory()?; + fn from_tar( + stream: impl Read, + temp_dir: temp::Dir, + io_executor: Box, + ) -> Result { let mut archive = tar::Archive::new(stream); // The rust-installer packages unpack to a directory called // $pkgname-$version-$target. Skip that directory when // unpacking. - unpack_without_first_dir( - &mut archive, - &temp_dir, - get_executor( - unpack_ram(IO_CHUNK_SIZE, dl_cfg.process.unpack_ram()?), - dl_cfg.process.io_thread_count()?, - ), - ) - .context("failed to extract package")?; + unpack_without_first_dir(&mut archive, &temp_dir, io_executor) + .context("failed to extract package")?; Self::new(temp_dir, false) } diff --git a/src/dist/manifestation.rs b/src/dist/manifestation.rs index 50d458adbb..b3fa9cedcd 100644 --- a/src/dist/manifestation.rs +++ b/src/dist/manifestation.rs @@ -10,6 +10,7 @@ use anyhow::{Context, Result, anyhow, bail}; use futures_util::stream::{FuturesUnordered, StreamExt}; use tracing::{info, warn}; +use crate::diskio::{IO_CHUNK_SIZE, get_executor, unpack_ram}; use crate::dist::component::{Components, DirectoryPackage, Transaction}; use crate::dist::config::Config; use crate::dist::download::{DownloadCfg, DownloadStatus, File}; @@ -423,7 +424,13 @@ impl Manifestation { // Install all the components in the installer let reader = utils::FileReaderWithProgress::new_file(&installer_file)?; - let package = DirectoryPackage::compressed(reader, CompressionKind::GZip, dl_cfg)?; + let temp_dir = dl_cfg.tmp_cx.new_directory()?; + let io_executor = get_executor( + unpack_ram(IO_CHUNK_SIZE, dl_cfg.process.unpack_ram()?), + dl_cfg.process.io_thread_count()?, + ); + let package = + DirectoryPackage::compressed(reader, CompressionKind::GZip, temp_dir, io_executor)?; for component in package.components() { tx = package.install(&self.installation, &component, None, tx)?; } @@ -687,8 +694,13 @@ impl<'a> ComponentBinary<'a> { self.status.installing(); let reader = utils::FileReaderWithProgress::new_file(&installer_file)?; + let temp_dir = self.download_cfg.tmp_cx.new_directory()?; + let io_executor = get_executor( + unpack_ram(IO_CHUNK_SIZE, self.download_cfg.process.unpack_ram()?), + self.download_cfg.process.io_thread_count()?, + ); let package = - DirectoryPackage::compressed(reader, self.binary.compression, self.download_cfg)?; + DirectoryPackage::compressed(reader, self.binary.compression, temp_dir, io_executor)?; // If the package doesn't contain the component that the // manifest says it does then somebody must be playing a joke on us. From 3ea37c9fae3fb2b55cdc7d181085594af3e62da3 Mon Sep 17 00:00:00 2001 From: Dirkjan Ochtman Date: Sun, 16 Nov 2025 14:10:06 +0100 Subject: [PATCH 20/20] dist: simplify ComponentBinary construction --- src/dist/manifest.rs | 8 +++++++ src/dist/manifestation.rs | 44 +++++++++++++++++---------------------- 2 files changed, 27 insertions(+), 25 deletions(-) diff --git a/src/dist/manifest.rs b/src/dist/manifest.rs index 7c32076951..10bd4c2adf 100644 --- a/src/dist/manifest.rs +++ b/src/dist/manifest.rs @@ -287,6 +287,14 @@ impl Manifest { Ok(toml::to_string(&self)?) } + pub(super) fn binary(&self, component: &Component) -> Result> { + let package = self.get_package(component.short_name_in_manifest())?; + let target_package = package.get_target(component.target.as_ref())?; + // We prefer the first format in the list, since the parsing of the + // manifest leaves us with the files/hash pairs in preference order. + Ok(target_package.bins.first()) + } + pub fn get_package(&self, name: &str) -> Result<&Package> { self.packages .get(name) diff --git a/src/dist/manifestation.rs b/src/dist/manifestation.rs index b3fa9cedcd..3ffd32b26e 100644 --- a/src/dist/manifestation.rs +++ b/src/dist/manifestation.rs @@ -152,31 +152,7 @@ impl Manifestation { let components = update .components_to_install .into_iter() - .filter_map(|component| { - let package = match new_manifest.get_package(component.short_name_in_manifest()) { - Ok(p) => p, - Err(e) => return Some(Err(e)), - }; - - let target_package = match package.get_target(component.target.as_ref()) { - Ok(tp) => tp, - Err(e) => return Some(Err(e)), - }; - - match target_package.bins.is_empty() { - // This package is not available, no files to download. - true => None, - // We prefer the first format in the list, since the parsing of the - // manifest leaves us with the files/hash pairs in preference order. - false => Some(Ok(ComponentBinary { - status: download_cfg.status_for(component.short_name(&new_manifest)), - component, - binary: &target_package.bins[0], - manifest: &new_manifest, - download_cfg, - })), - } - }) + .filter_map(|component| ComponentBinary::new(component, &new_manifest, download_cfg)) .collect::>>()?; const DEFAULT_CONCURRENT_DOWNLOADS: usize = 2; @@ -647,6 +623,24 @@ struct ComponentBinary<'a> { } impl<'a> ComponentBinary<'a> { + fn new( + component: Component, + manifest: &'a Manifest, + download_cfg: &'a DownloadCfg<'a>, + ) -> Option> { + Some(Ok(ComponentBinary { + binary: match manifest.binary(&component) { + Ok(Some(b)) => b, + Ok(None) => return None, + Err(e) => return Some(Err(e)), + }, + status: download_cfg.status_for(component.short_name(manifest)), + component, + manifest, + download_cfg, + })) + } + async fn download(self, max_retries: usize) -> Result<(Self, File)> { use tokio_retry::{RetryIf, strategy::FixedInterval};