From 527f0813c40263982e01c11137042d713179d0a0 Mon Sep 17 00:00:00 2001 From: Dirkjan Ochtman Date: Sun, 21 Sep 2025 15:23:10 +0200 Subject: [PATCH 1/6] dist: avoid cloning components Vec --- src/dist/manifestation.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/dist/manifestation.rs b/src/dist/manifestation.rs index fd890b1a18..c30480836b 100644 --- a/src/dist/manifestation.rs +++ b/src/dist/manifestation.rs @@ -173,7 +173,7 @@ impl Manifestation { .unwrap_or(DEFAULT_MAX_RETRIES); info!("downloading component(s)"); - for (component, _, url, _) in components.clone() { + for (component, _, url, _) in &components { (download_cfg.notify_handler)(Notification::DownloadingComponent( &component.short_name(new_manifest), &self.target_triple, From 097a59e8a3399f7ab1c62323a814e54cc51b3c86 Mon Sep 17 00:00:00 2001 From: Dirkjan Ochtman Date: Sun, 21 Sep 2025 15:23:32 +0200 Subject: [PATCH 2/6] dist: avoid unnecessary type annotations --- 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 c30480836b..99feda3d04 100644 --- a/src/dist/manifestation.rs +++ b/src/dist/manifestation.rs @@ -153,8 +153,8 @@ impl Manifestation { let altered = tmp_cx.dist_server != DEFAULT_DIST_SERVER; // Download component packages and validate hashes - let mut things_to_install: Vec<(Component, CompressionKind, File)> = Vec::new(); - let mut things_downloaded: Vec = Vec::new(); + let mut things_to_install = Vec::new(); + let mut things_downloaded = Vec::new(); let components = update.components_urls_and_hashes(new_manifest)?; let components_len = components.len(); From 834dcb3ad9e848f66606c08e21a8ec98147c9fb3 Mon Sep 17 00:00:00 2001 From: Dirkjan Ochtman Date: Sun, 21 Sep 2025 15:29:41 +0200 Subject: [PATCH 3/6] dist: avoid passing through arguments --- src/dist/manifestation.rs | 21 ++++++++++----------- 1 file changed, 10 insertions(+), 11 deletions(-) diff --git a/src/dist/manifestation.rs b/src/dist/manifestation.rs index 99feda3d04..4194acc0c3 100644 --- a/src/dist/manifestation.rs +++ b/src/dist/manifestation.rs @@ -189,10 +189,9 @@ impl Manifestation { async move { let _permit = sem.acquire().await.unwrap(); self.download_component( - component, - format, - url, - hash, + &component, + &url, + &hash, altered, tmp_cx, download_cfg, @@ -200,6 +199,7 @@ impl Manifestation { new_manifest, ) .await + .map(|downloaded| (component, format, downloaded, hash)) } }); if components_len > 0 { @@ -551,22 +551,21 @@ impl Manifestation { #[allow(clippy::too_many_arguments)] async fn download_component( &self, - component: Component, - format: CompressionKind, - url: String, - hash: String, + component: &Component, + url: &str, + hash: &str, altered: bool, tmp_cx: &temp::Context, download_cfg: &DownloadCfg<'_>, max_retries: usize, new_manifest: &Manifest, - ) -> Result<(Component, CompressionKind, File, String)> { + ) -> Result { use tokio_retry::{RetryIf, strategy::FixedInterval}; let url = if altered { url.replace(DEFAULT_DIST_SERVER, tmp_cx.dist_server.as_str()) } else { - url + url.to_owned() }; let url_url = utils::parse_url(&url)?; @@ -589,7 +588,7 @@ impl Manifestation { .await .with_context(|| RustupError::ComponentDownloadFailed(component.name(new_manifest)))?; - Ok((component, format, downloaded_file, hash)) + Ok(downloaded_file) } } From 4ec3e0ad778dbed5e596e0da5e8663c2b00e5867 Mon Sep 17 00:00:00 2001 From: Dirkjan Ochtman Date: Sun, 21 Sep 2025 15:33:31 +0200 Subject: [PATCH 4/6] dist: introduce ComponentBinary type --- src/dist/manifestation.rs | 92 ++++++++++++++++++++------------------- 1 file changed, 47 insertions(+), 45 deletions(-) diff --git a/src/dist/manifestation.rs b/src/dist/manifestation.rs index 4194acc0c3..b5465c144a 100644 --- a/src/dist/manifestation.rs +++ b/src/dist/manifestation.rs @@ -17,7 +17,7 @@ use crate::dist::component::{ }; use crate::dist::config::Config; use crate::dist::download::{DownloadCfg, File}; -use crate::dist::manifest::{Component, CompressionKind, Manifest, TargetedPackage}; +use crate::dist::manifest::{Component, CompressionKind, HashedBinary, Manifest, TargetedPackage}; use crate::dist::notifications::*; use crate::dist::prefix::InstallPrefix; use crate::dist::temp; @@ -173,44 +173,41 @@ impl Manifestation { .unwrap_or(DEFAULT_MAX_RETRIES); info!("downloading component(s)"); - for (component, _, url, _) in &components { + for bin in &components { (download_cfg.notify_handler)(Notification::DownloadingComponent( - &component.short_name(new_manifest), + &bin.component.short_name(new_manifest), &self.target_triple, - component.target.as_ref(), - &url, + bin.component.target.as_ref(), + &bin.binary.url, )); } let semaphore = Arc::new(Semaphore::new(concurrent_downloads)); - let component_stream = - tokio_stream::iter(components.into_iter()).map(|(component, format, url, hash)| { - let sem = semaphore.clone(); - async move { - let _permit = sem.acquire().await.unwrap(); - self.download_component( - &component, - &url, - &hash, - altered, - tmp_cx, - download_cfg, - max_retries, - new_manifest, - ) - .await - .map(|downloaded| (component, format, downloaded, hash)) - } - }); + let component_stream = tokio_stream::iter(components.into_iter()).map(|bin| { + let sem = semaphore.clone(); + async move { + let _permit = sem.acquire().await.unwrap(); + self.download_component( + &bin, + altered, + tmp_cx, + download_cfg, + max_retries, + new_manifest, + ) + .await + .map(|downloaded| (bin, downloaded)) + } + }); if components_len > 0 { let results = component_stream .buffered(components_len) .collect::>() .await; for result in results { - let (component, format, downloaded_file, hash) = result?; - things_downloaded.push(hash); - things_to_install.push((component, format, downloaded_file)); + let (bin, downloaded_file) = result?; + things_downloaded.push(bin.binary.hash.clone()); + things_to_install.push((bin.component, bin.binary.compression, downloaded_file)); } } @@ -548,12 +545,9 @@ impl Manifestation { Ok(tx) } - #[allow(clippy::too_many_arguments)] async fn download_component( &self, - component: &Component, - url: &str, - hash: &str, + component: &ComponentBinary<'_>, altered: bool, tmp_cx: &temp::Context, download_cfg: &DownloadCfg<'_>, @@ -563,16 +557,19 @@ impl Manifestation { use tokio_retry::{RetryIf, strategy::FixedInterval}; let url = if altered { - url.replace(DEFAULT_DIST_SERVER, tmp_cx.dist_server.as_str()) + component + .binary + .url + .replace(DEFAULT_DIST_SERVER, tmp_cx.dist_server.as_str()) } else { - url.to_owned() + component.binary.url.clone() }; let url_url = utils::parse_url(&url)?; let downloaded_file = RetryIf::spawn( FixedInterval::from_millis(0).take(max_retries), - || download_cfg.download(&url_url, &hash), + || download_cfg.download(&url_url, &component.binary.hash), |e: &anyhow::Error| { // retry only known retriable cases match e.downcast_ref::() { @@ -586,7 +583,9 @@ impl Manifestation { }, ) .await - .with_context(|| RustupError::ComponentDownloadFailed(component.name(new_manifest)))?; + .with_context(|| { + RustupError::ComponentDownloadFailed(component.component.name(new_manifest)) + })?; Ok(downloaded_file) } @@ -781,10 +780,10 @@ impl Update { } /// Map components to urls and hashes - fn components_urls_and_hashes( - &self, - new_manifest: &Manifest, - ) -> Result> { + fn components_urls_and_hashes<'a>( + &'a self, + new_manifest: &'a Manifest, + ) -> Result>> { let mut components_urls_and_hashes = Vec::new(); for component in &self.components_to_install { let package = new_manifest.get_package(component.short_name_in_manifest())?; @@ -796,14 +795,17 @@ impl Update { } // We prefer the first format in the list, since the parsing of the // manifest leaves us with the files/hash pairs in preference order. - components_urls_and_hashes.push(( - component.clone(), - target_package.bins[0].compression, - target_package.bins[0].url.clone(), - target_package.bins[0].hash.clone(), - )); + components_urls_and_hashes.push(ComponentBinary { + component, + binary: &target_package.bins[0], + }); } Ok(components_urls_and_hashes) } } + +struct ComponentBinary<'a> { + component: &'a Component, + binary: &'a HashedBinary, +} From b50f66a91e389223b19e02471227b6052edfee6b Mon Sep 17 00:00:00 2001 From: Dirkjan Ochtman Date: Sun, 21 Sep 2025 15:35:51 +0200 Subject: [PATCH 5/6] dist: detach download_component() from Manifestation --- src/dist/manifestation.rs | 101 +++++++++++++++++--------------------- 1 file changed, 46 insertions(+), 55 deletions(-) diff --git a/src/dist/manifestation.rs b/src/dist/manifestation.rs index b5465c144a..746a8cc630 100644 --- a/src/dist/manifestation.rs +++ b/src/dist/manifestation.rs @@ -187,16 +187,9 @@ impl Manifestation { let sem = semaphore.clone(); async move { let _permit = sem.acquire().await.unwrap(); - self.download_component( - &bin, - altered, - tmp_cx, - download_cfg, - max_retries, - new_manifest, - ) - .await - .map(|downloaded| (bin, downloaded)) + bin.download(altered, tmp_cx, download_cfg, max_retries, new_manifest) + .await + .map(|downloaded| (bin, downloaded)) } }); if components_len > 0 { @@ -544,51 +537,6 @@ impl Manifestation { Ok(tx) } - - async fn download_component( - &self, - component: &ComponentBinary<'_>, - altered: bool, - tmp_cx: &temp::Context, - download_cfg: &DownloadCfg<'_>, - max_retries: usize, - new_manifest: &Manifest, - ) -> Result { - use tokio_retry::{RetryIf, strategy::FixedInterval}; - - let url = if altered { - component - .binary - .url - .replace(DEFAULT_DIST_SERVER, tmp_cx.dist_server.as_str()) - } else { - component.binary.url.clone() - }; - - let url_url = utils::parse_url(&url)?; - - let downloaded_file = RetryIf::spawn( - FixedInterval::from_millis(0).take(max_retries), - || download_cfg.download(&url_url, &component.binary.hash), - |e: &anyhow::Error| { - // retry only known retriable cases - match e.downcast_ref::() { - Some(RustupError::BrokenPartialFile) - | Some(RustupError::DownloadingFile { .. }) => { - (download_cfg.notify_handler)(Notification::RetryingDownload(&url)); - true - } - _ => false, - } - }, - ) - .await - .with_context(|| { - RustupError::ComponentDownloadFailed(component.component.name(new_manifest)) - })?; - - Ok(downloaded_file) - } } #[derive(Debug)] @@ -809,3 +757,46 @@ struct ComponentBinary<'a> { component: &'a Component, binary: &'a HashedBinary, } + +impl<'a> ComponentBinary<'a> { + async fn download( + &self, + altered: bool, + tmp_cx: &temp::Context, + download_cfg: &DownloadCfg<'_>, + max_retries: usize, + new_manifest: &Manifest, + ) -> Result { + use tokio_retry::{RetryIf, strategy::FixedInterval}; + + let url = if altered { + self.binary + .url + .replace(DEFAULT_DIST_SERVER, tmp_cx.dist_server.as_str()) + } else { + self.binary.url.clone() + }; + + let url_url = utils::parse_url(&url)?; + + let downloaded_file = RetryIf::spawn( + FixedInterval::from_millis(0).take(max_retries), + || download_cfg.download(&url_url, &self.binary.hash), + |e: &anyhow::Error| { + // retry only known retriable cases + match e.downcast_ref::() { + Some(RustupError::BrokenPartialFile) + | Some(RustupError::DownloadingFile { .. }) => { + (download_cfg.notify_handler)(Notification::RetryingDownload(&url)); + true + } + _ => false, + } + }, + ) + .await + .with_context(|| RustupError::ComponentDownloadFailed(self.component.name(new_manifest)))?; + + Ok(downloaded_file) + } +} From 1a2cc72afee6b4cf88b1b3ae4d07cf8775030449 Mon Sep 17 00:00:00 2001 From: Dirkjan Ochtman Date: Sun, 21 Sep 2025 15:40:02 +0200 Subject: [PATCH 6/6] dist: extract URL alteration from download() method --- src/dist/manifestation.rs | 30 +++++++++++++++--------------- 1 file changed, 15 insertions(+), 15 deletions(-) diff --git a/src/dist/manifestation.rs b/src/dist/manifestation.rs index 746a8cc630..edc6d42580 100644 --- a/src/dist/manifestation.rs +++ b/src/dist/manifestation.rs @@ -11,6 +11,7 @@ use futures_util::stream::StreamExt; use std::sync::Arc; use tokio::sync::Semaphore; use tracing::info; +use url::Url; use crate::dist::component::{ Components, Package, TarGzPackage, TarXzPackage, TarZStdPackage, Transaction, @@ -187,7 +188,17 @@ impl Manifestation { let sem = semaphore.clone(); async move { let _permit = sem.acquire().await.unwrap(); - bin.download(altered, tmp_cx, download_cfg, max_retries, new_manifest) + let url = if altered { + utils::parse_url( + &bin.binary + .url + .replace(DEFAULT_DIST_SERVER, tmp_cx.dist_server.as_str()), + )? + } else { + utils::parse_url(&bin.binary.url)? + }; + + bin.download(&url, download_cfg, max_retries, new_manifest) .await .map(|downloaded| (bin, downloaded)) } @@ -761,33 +772,22 @@ struct ComponentBinary<'a> { impl<'a> ComponentBinary<'a> { async fn download( &self, - altered: bool, - tmp_cx: &temp::Context, + url: &Url, download_cfg: &DownloadCfg<'_>, max_retries: usize, new_manifest: &Manifest, ) -> Result { use tokio_retry::{RetryIf, strategy::FixedInterval}; - let url = if altered { - self.binary - .url - .replace(DEFAULT_DIST_SERVER, tmp_cx.dist_server.as_str()) - } else { - self.binary.url.clone() - }; - - let url_url = utils::parse_url(&url)?; - let downloaded_file = RetryIf::spawn( FixedInterval::from_millis(0).take(max_retries), - || download_cfg.download(&url_url, &self.binary.hash), + || download_cfg.download(url, &self.binary.hash), |e: &anyhow::Error| { // retry only known retriable cases match e.downcast_ref::() { Some(RustupError::BrokenPartialFile) | Some(RustupError::DownloadingFile { .. }) => { - (download_cfg.notify_handler)(Notification::RetryingDownload(&url)); + (download_cfg.notify_handler)(Notification::RetryingDownload(url.as_str())); true } _ => false,